-
-
Notifications
You must be signed in to change notification settings - Fork 3k
fix: hashing of chunks #14811
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
fix: hashing of chunks #14811
Conversation
|
| enforce: 'pre', | ||
|
|
||
| augmentChunkHash() { | ||
| return Date.now().toString(); |
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.
Unfortunately, I couldn't find a way to discriminate the hashing to a certain module.
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.
Did you try moving this into a plugin the test uses? I had a similar issue in another test and was able to add this to the test, so it doesn't affect real builds (where I assume it's not actually needed)
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 had this issue inside an integration test (as you can see from the code); they operate on the same fixture, but with a different configuration.
If this issue happens in integration tests, it means it could happen in production too. Which means that a production application might end up using outdated chunks.
I don't think the issue is the test
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.
You can add it as a plugin each time you configure the fixture. I'll approve but am going to add an issue to the board to look and see if we can move it after tests are passing.
The reason I flag this is because if this happens in the client build it will produce new files for each build, so the HTTP cache won't be used. Not a big deal at the moment, once tests are passing we can revisit.
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 managed to reduce the augmentation of the hash to only the pretender entry point. Please review again
| }); | ||
| await fixture.build(); | ||
| }); | ||
| describe('with [prefix-other-locales]', () => { |
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 just extracted some describe and moved them to the top-level. This will help with adding .only to fewer blocks
matthewp
left a comment
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.
Try putting the augmentHash in the test as a plugin there
| enforce: 'pre', | ||
|
|
||
| augmentChunkHash() { | ||
| return Date.now().toString(); |
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.
Did you try moving this into a plugin the test uses? I had a similar issue in another test and was able to add this to the test, so it doesn't affect real builds (where I assume it's not actually needed)
|
Aside from linting it lgtm |
Co-authored-by: Emanuele Stoppa <[email protected]> Co-authored-by: Samuel Macleod <[email protected]> Co-authored-by: Alexander Niebuhr <[email protected]> Co-authored-by: ascorbic <[email protected]> Co-authored-by: matthewp <[email protected]> Co-authored-by: Florian Lefebvre <[email protected]> Co-authored-by: Matt Kane <[email protected]> Co-authored-by: florian-lefebvre <[email protected]> Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: Matthew Phillips <[email protected]> Co-authored-by: Matthew Phillips <[email protected]> Co-authored-by: Sarah Rainsberger <[email protected]> Co-authored-by: Erika <[email protected]> Co-authored-by: Hayato Hasegawa <[email protected]> fix: creation of routes list in build (#14325) Fix unit tests for environment API (#14394) fixture (#14599) fix: organize imports and formatting in cloudflare handler (#14697) fix: resolve environment-api build issues for client & CSS (#14770) Fixes CSS when both static/client styles exist (#14790) fix: flag `serverLike` incorrectly computed (#14792) fix: redirects and i18n routing (#14797) fix: logger level and static paths params (#14799) Fix image tests crashing (#14803) fix(environment): Error overlay not working (#14816) fix: hashing of chunks (#14811) Fix before-hydration script emission for environment-api (#14818) fix: node leak into content collections (#14820) fix: avoid `node:path` (#14827) Fix API route tests (#14825) Fix unit tests (#14832) fix: server islands build (#14830) Fix astro-component-bundling tests (#14840) Fix astro-component-bundling tests (#14837) Fix config alias by using resolve.alias config (#14828) fix: container and routing tests (#14852) fix: respect user vite config on sourcemaps (#14847) fix: remove useless tests (#14854) Fixes assetQueryParams and public folder tests (#14853) fix: renderError fallback (#14855) Fix server-islands data being used across tests (#14857) fix: incorrect configuration in tests (#14858) Fixes for tests in environment branch (#14865) Fixes for e2e tests in environment branch (#14870) fix(test): svelte async rendering (#14886) fix(cloudflare/vercel/netlify): some tests pass and excluded others (#14892) Fixes for node integrations tests (#14900) Fix @astrojs/db tests (#14899) fix(environment): Prevent Vite from interpreting injected sourcemap comments as actual sourcemaps (#14921) Fix remaining cloudflare tests (#14912) fix(vite): import mergeConfig from direct file to avoid dynamic import being processed by Vite (#14939) Fix astro:env tests in Cloudflare (#14925)
Changes
The chunks emitted during the build had the same hash in some cases (don't know how, apologies). This resulted in some tests picking the manifest that belongs to another test that run before it. causing all sorts of false positives.
Adding
augmentChunkHashto thevirtual:astro:manifestseems to fix those testsTesting
All test of
i18n-routing.test.jsnow passDocs