Repository navigation
Fix the markets recipe doc links and the Deleted Files links - #4291
Conversation
|
Oxygen deployed a preview of your
Learn more about Hydrogen's GitHub integration. |
fredericoo
left a comment
There was a problem hiding this comment.
LGTM! Checked that every link except the Deleted Files ones (see inline) resolves at 68f62c4, and that render and generateLLMsFiles reproduce both files byte-for-byte from the new recipe.yaml.
| - [templates/skeleton/app/routes/policies.$handle.tsx](https://github.com/Shopify/hydrogen/blob/1040066d20b52667756fd1ebffd8607602a735b4/templates/skeleton/templates/skeleton/app/routes/policies.$handle.tsx) | ||
| - [templates/skeleton/app/routes/policies._index.tsx](https://github.com/Shopify/hydrogen/blob/1040066d20b52667756fd1ebffd8607602a735b4/templates/skeleton/templates/skeleton/app/routes/policies._index.tsx) | ||
| - [templates/skeleton/app/routes/search.tsx](https://github.com/Shopify/hydrogen/blob/1040066d20b52667756fd1ebffd8607602a735b4/templates/skeleton/templates/skeleton/app/routes/search.tsx) | ||
| - [templates/skeleton/app/routes/account.$.tsx](https://github.com/Shopify/hydrogen/blob/68f62c423b5382cb6a98a4f0000d656bf98b9c13/templates/skeleton/templates/skeleton/app/routes/account.$.tsx) |
There was a problem hiding this comment.
non-blocking: these 20 Deleted Files links still 404, because the path has templates/skeleton/ twice.
Reproduced: none of the 20 paths exist in the 68f62c4 tree; the same paths with a single templates/skeleton/ all do.
deletedFiles entries already start with templates/skeleton/, and render.ts adds it again. This predates the PR and also affects the express and multipass READMEs, so it's out of scope here. Worth a follow-up in render.ts:
// before
`${linkPrefix}/templates/skeleton/${file}`
// after
`${linkPrefix}/${file}`The markets recipe README and LLM prompt link to files at the commit in recipe.yaml. That commit was older than #4278, so the links to sanitizeHtml.ts returned 404. Set the commit to the #4278 merge commit and render the README and prompt again. The Deleted Files links also returned 404. The deletedFiles paths already start with templates/skeleton/, and render.ts added that prefix again. Remove the extra prefix, and fix the same links in the express and multipass READMEs.
dede094 to
8dedcd0
Compare
TL;DR: Some links in the cookbook docs return 404. The markets links to the new
sanitizeHtml.tspoint to a commit that is older than the file, and every "Deleted Files" link hastemplates/skeleton/twice. This PR fixes both.The rendered README and LLM prompt build every file link from the
commitfield inrecipe.yaml. For markets, that field was1040066d, which is older than #4278. The shopify.dev page uses the same links.Before
https://github.com/Shopify/hydrogen/blob/1040066d…/cookbook/recipes/markets/ingredients/templates/skeleton/app/lib/sanitizeHtml.tsreturns 404.https://github.com/Shopify/hydrogen/blob/<commit>/templates/skeleton/templates/skeleton/app/routes/_index.tsxreturns 404.After
sanitizeHtml.tslink opens the sanitizer./templates/skeleton/once and open the file at the pinned commit.What this changes
commitincookbook/recipes/markets/recipe.yamlto the Sanitize merchant-managed HTML in the markets recipe #4278 merge commit, and renders the markets README and LLM prompt again.cookbook/src/lib/render.ts, stops addingtemplates/skeleton/to Deleted Files links. ThedeletedFilespaths in each recipe already start with it.express(59 links) andmultipass(1 link) READMEs. The changes in those two files touch only these links. A full render of either README would also bring in other recipe changes that nobody has rendered yet, so this PR does not do one.Developer impact
No changeset. The cookbook does not ship in a package.
Out of scope
shop/world. A separate PR updates it.pnpm run cookbook regeneratesetscommitautomatically, but it runsapplyfirst, andapplyfails on the marketspackage.jsonpatch. This PR setscommitby hand.How to Test
cookbook/recipes/markets/README.mdon this branch.app/lib/sanitizeHtml.tslink in the ingredients table.