Add Readmes for public and app/assets directories #1934
Conversation
| @@ -0,0 +1,7 @@ | |||
| Use the `/public` folder to store static files that **shouldn't be processed at build time**. | |||
There was a problem hiding this comment.
This file here will be publicly available in domain.com/readme.md if they don't remove it manually. Perhaps we should inform about assets in the root template readme instead?
There was a problem hiding this comment.
Ha, I wondered about that. Could move it out for sure, although I think having it in context has some value. Is there some way to configure an ignore rule?
There was a problem hiding this comment.
Just testing this out, and of course the build process does produce dist/client/readme.md. But it throws a 404 in Oxygen: https://01htfeq3ddq1kvwp4adw76d3tf-bdc62392032d96aafe24.myshopify.dev/readme.md — wondering if the CDN just ignores certain file types by default or if there's something else going on.
There was a problem hiding this comment.
That's the worker domain. I guess it doesn't proxy .md files to the CDN 🤔
There was a problem hiding this comment.
@jgodson confirmed: https://github.com/Shopify/oxygen-workers/blob/fc213b66075fdd59a74954317fa77b4794d91a64/packages/routing-worker/src/assets.ts#L23-L44
@gfscott is the desire to give this folder context on Github with the readme file? Or also to give context inside vscode after you've scaffolded a new project? If it's just Github, we could update the CLI to not copy it over on new projects.
There was a problem hiding this comment.
Nm, apparently there's a time in the future where this will change, and our proxy logic will not filter assets like this, so then this file would start to show up.
There was a problem hiding this comment.
is the desire to give this folder context on Github with the readme file? Or also to give context inside vscode after you've scaffolded a new project?
More the latter. Why ask them to google it or pore over the docs when we could just say what the folder is, right in context? Seems to me like the canonical role of a readme file.
That said, example.com/readme.md is not a desirable outcome 😅
Is there some exclude/ignore list we can give to Vite at build time?
There was a problem hiding this comment.
I think we'd need to update the deploy command to exclude them somehow. Maybe an ignore file. I don't think we will have time to do this for the release this week.
There was a problem hiding this comment.
Oh yeah, this is not urgent at all
Co-authored-by: Fran Dios <frankdiox@gmail.com>
|
Oxygen deployed a preview of your
Learn more about Hydrogen's GitHub integration. |
We've observed some confusion in the wild about the two directories where Hydrogen projects can keep static assets:
/publicand/app/assets.This PR adds a readme to each directory with some contextual detail on what happens to the assets stored there. Very open to edits, fact-checks, etc.
Checklist
I've added tests to cover my changesI've added or updated the documentation