Skip to content
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

embedded assets #1681

Merged
merged 5 commits into from
Sep 29, 2024
Merged

embedded assets #1681

merged 5 commits into from
Sep 29, 2024

Conversation

mbostock
Copy link
Member

Allows additional paths, backed either by static files or data loaders, to be included in the built output under stable URLs. For example it could be /robots.txt or it could be /product/413021.svg.

@mbostock mbostock requested a review from Fil September 24, 2024 21:32
src/search.ts Outdated Show resolved Hide resolved
@Fil
Copy link
Contributor

Fil commented Sep 25, 2024

I've tested, building works like a charm (even from archives… which was expected but nonetheless nice to see).

Proto-documentation (to go at the bottom of files.md):

<a name="static-assets" href="#static-assets"></a>Static assets listed as
[**dynamicPaths**](./config#dynamic-paths) will be built at the given path instead of
being promoted to content-hashed file attachments.
This allows you to create the handful of (non HTML) ressources that must be served from an
invariable URL, such as an ATOM or RSS feed. (If you reference these assets from a link,
set `rel=external` or use an absolute URL to avoid duplication.)

What remains to do:

  • avoid promoting a static asset to a file attachment
    • e.g. <link rel="alternate" href="./feed.xml"> should check if /feed.xml is a dynamicPath before promoting it to _file/feed.12345678.xml); then the last sentence of the documentation could be removed. (not obvious to me how to do this).
  • serve static assets in preview

@mbostock
Copy link
Member Author

mbostock commented Sep 25, 2024

avoid promoting a static asset to a file attachment

I don’t think we should do this. If you link to it and we detect it, you should get the content-hashed version (even if that means the file exists twice). If you don’t want that, you need to use rel="alternate external" to opt-out (but not sure if that works for RSS detection… hopefully? We could strip the external annotation during build maybe).

Also don’t think we need to serve these during preview.

@Fil
Copy link
Contributor

Fil commented Sep 25, 2024

OK for not serving them, but is this more like "not a priority" or are there arguments against it?

@mbostock
Copy link
Member Author

I guess I’m thinking about these assets as intended for external consumption (for embedding), not for use internally within a Framework application. But you’re right, the preview server could support it, just like it supports CORS so you can test your embedded modules locally…

@mbostock
Copy link
Member Author

I think the documentation should go in /embeds, not /files. That’s the one thing left for this PR IMO.

@mbostock mbostock marked this pull request as ready for review September 29, 2024 04:35
@mbostock
Copy link
Member Author

I added documentation in #1692, forgetting that I hadn’t merged this yet. 🤦 Any blockers for this PR @Fil?

@mbostock mbostock enabled auto-merge (squash) September 29, 2024 04:36
@mbostock mbostock merged commit e08f0b9 into main Sep 29, 2024
4 checks passed
@mbostock mbostock deleted the mbostock/embedded-assets branch September 29, 2024 08:25
@Fil Fil mentioned this pull request Sep 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants