Skip to content

FileAttachments: Stdlib Implementation #128

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 23 commits into from
Oct 29, 2019
Merged

FileAttachments: Stdlib Implementation #128

merged 23 commits into from
Oct 29, 2019

Conversation

jashkenas
Copy link
Contributor

@jashkenas jashkenas commented Oct 18, 2019 •
edited
Loading

... for use in UI, API (for downloads), and embeds.

  • UI: resolve() function posts messages up to the parent frame to get signed URLs for private files.
  • API: resolve() function is compiled with relative paths to downloaded files in the folder hierarchy.
  • Embeds: resolve() function is compiled into the ES Modules, with absolute URLs of published files.

Update

  • Embeds: resolve() function is compiled into the generated ES Modules, with a map from name to absolute public (CloudFront), or private (currently CloudFront, future API) URLs.
  • Downloads: resolve() function is compiled into the generated ES Modules, with a map from name to relative path on disk.
  • Imports: Identical to Embeds, compiled with absolute URLs.
  • UI: Passes in a resolve() function to this implementation that is capable of asking the parent frame (which may ask the server) for the URL for a given file.

@jashkenas jashkenas mentioned this pull request Oct 18, 2019
Merged
visnup
visnup reviewed Oct 21, 2019
View reviewed changes
@jashkenas jashkenas requested a review from mbostock October 23, 2019 20:16
@jashkenas jashkenas changed the title Basic FileAttachment implementation, with "resolve" function FileAttachments: Runtime Implementation Oct 23, 2019
@jashkenas jashkenas changed the title FileAttachments: Runtime Implementation FileAttachments: Stdlib Implementation Oct 23, 2019
@jashkenas jashkenas marked this pull request as ready for review October 23, 2019 20:19
mbostock
mbostock approved these changes Oct 23, 2019
View reviewed changes
Copy link
Member

@mbostock mbostock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, but I think we should wait until we’re ready to ship before merging it to master and publishing to npm. In the meantime, let’s keep this working branch open.

yarn.lock
integrity sha512-zfat3WTZRZmh7jCrTIhg4zZvivA0DZvez8lZq0JwYG9aW8LcSUFO4eiPnL5F2MolHcLE8CLfEt06sPP8N7y3AQ==
d3-require@^1.2.4:
version "1.2.4"
resolved "https://registry.npmjs.org/d3-require/-/d3-require-1.2.4.tgz#59afc591d5089f99fecd8c45ef7539e1fee112b3"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yarnpkg.com?

@mbostock mbostock mentioned this pull request Oct 29, 2019
Closed
@mbostock
Copy link
Member

We should add a section to the README before this lands.

@mbostock mbostock merged commit 4fd1f08 into master Oct 29, 2019
@mbostock mbostock deleted the file-attachments branch October 29, 2019 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@visnup visnup visnup approved these changes

@mbostock mbostock mbostock approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jashkenas @mbostock @visnup @rreusser