-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support client-side exports and one-time export tokens #1240
Conversation
* - `() => Promise<URL>` Provider generates single-use export URLs (i.e. signed one-time tokens) | ||
* - `() => Promise<Blob>` Export is to be generated client-side | ||
* - `undefined` Export scenario is not supported | ||
*/ | ||
public getExportURL?<D extends Dataset<ArrayShape>>( | ||
dataset: D, | ||
selection: string | undefined, | ||
value: Value<D>, | ||
format: ExportFormat |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm tempted to move the format
first. What do you think? If you agree, I'll do that in a separate PR.
async function handleBtnClick(getURLOrBlob: () => Promise<URL | Blob>) { | ||
const anchor = document.createElement('a'); | ||
anchor.download = filename; | ||
document.body.append(anchor); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To start a download for which you don't know the URL right away, the idea is to add a temporary link to the DOM (invisibly), set its href
once the URL is known, and click on it programmatically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, that is convoluted. Perhaps add a small comment to state the idea ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, let me know if it's clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep 👍
In the future, we may want to improve the APIs of the provider components (especially We can also consider bringing some client-side export utilities into However, in both cases, I would prefer to see consumer implementations first. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Promising 💯
async function handleBtnClick(getURLOrBlob: () => Promise<URL | Blob>) { | ||
const anchor = document.createElement('a'); | ||
anchor.download = filename; | ||
document.body.append(anchor); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, that is convoluted. Perhaps add a small comment to state the idea ?
const label = `Export to ${format.toUpperCase()}`; | ||
const filename = `data.${format}`; | ||
|
||
async function handleBtnClick(getURLOrBlob: () => Promise<URL | Blob>) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function could be moved below the check url instanceof URL
as it will not be used in this case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functions declarations are hoisted, so this won't do what you think 😉: https://developer.mozilla.org/en-US/docs/Glossary/Hoisting
Agreed. All the bricks are there but we must first build something with them before deciding on the future abstractions. |
52bea80
to
15ad02e
Compare
/approve |
Update Cypress reference snapshots
Fix #1063
Providers that implement
getExportURL
can now return an async function that gets called when the user actually clicks on an export entry. This function can return eitherBlob
orURL
to support two main use cases:Blob
for generating client-side exports as per Allow client-generated exports of datasets #1063URL
for authenticated back-ends, as per https://gitlab.esrf.fr/ui/daiquiri/-/merge_requests/538#note_195991 (i.e. when a one-time token has to be fetched from an authenticated endpoint)Note that this is not restrictive. Data provider implementations are free to do whatever they want - e.g.:
data:
URI instead of aBlob
(with the risk of reaching the URL length limit);GET
request directly to an authenticated export endpoint (and return aBlob
) instead of requesting/returning a one-time token.Now to actually resolve #1063 and https://gitlab.esrf.fr/ui/daiquiri/-/merge_requests/538#note_195991, I've added a way for consumers of the providers to pass their own implementations of
getExportURL
.