Skip to content

fix(core): append assetQueryParams to inter-chunk JS imports (#15964)#16110

Merged
matthewp merged 2 commits intowithastro:mainfrom
tmimmanuel:fix/skew-protection-inter-chunk-imports
Mar 31, 2026
Merged

fix(core): append assetQueryParams to inter-chunk JS imports (#15964)#16110
matthewp merged 2 commits intowithastro:mainfrom
tmimmanuel:fix/skew-protection-inter-chunk-imports

Conversation

@tmimmanuel
Copy link
Copy Markdown
Contributor

Changes

Before:

import{g as t}from"./shared.DuaLvmHQ.js"

After:

import{g as t}from"./shared.DuaLvmHQ.js?dpl=deploy-abc123"

Closes #15964

Testing

  • New test: appends assetQueryParams to relative imports inside client JS chunks
  • All 5 existing asset-query-params tests pass
  • Built JS chunks verified to contain ?dpl= on relative from"./..." imports
  • Plugin is a no-op when no assetQueryParams configured (no regression for non-Vercel builds)

Docs

No docs changes needed.

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 26, 2026

🦋 Changeset detected

Latest commit: ebf6975

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Mar 26, 2026
@matthewp
Copy link
Copy Markdown
Contributor

can we use es-module-lexer for this? we already use it in some places like here:

Definitely want to make sure we're getting both static and dynamic imports.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Mar 26, 2026

Merging this PR will not alter performance

✅ 18 untouched benchmarks


Comparing tmimmanuel:fix/skew-protection-inter-chunk-imports (ebf6975) with main (47a394d)1

Open in CodSpeed

Footnotes

  1. No successful run was found on main (15f1649) during the generation of this report, so 47a394d was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@tmimmanuel
Copy link
Copy Markdown
Contributor Author

can we use es-module-lexer for this? we already use it in some places like here:

Definitely want to make sure we're getting both static and dynamic imports.

@matthewp Thanks for the review! I'll refactor to use es-module-lexer instead of regex — makes sense to keep it consistent with the rest of the codebase. Will update the PR shortly.

@tmimmanuel tmimmanuel closed this Mar 26, 2026
@tmimmanuel tmimmanuel reopened this Mar 26, 2026
@tmimmanuel tmimmanuel force-pushed the fix/skew-protection-inter-chunk-imports branch from 1453e8d to 6256749 Compare March 26, 2026 21:22
@tmimmanuel
Copy link
Copy Markdown
Contributor Author

Hi, @matthewp
I think current test failures are not from my code change.
Can you review again please? Thanks. 🙏

@matthewp
Copy link
Copy Markdown
Contributor

No it's a flaky test, I'll rerun it.

@matthewp
Copy link
Copy Markdown
Contributor

Fixes #14775

@tmimmanuel
Copy link
Copy Markdown
Contributor Author

Hi @matthewp
Thanks for your review. CI tests are passed. Please review again.

@matthewp
Copy link
Copy Markdown
Contributor

@tmimmanuel Code looks good, can you add a changeset? pnpm run changeset. This page explains how to write them: https://contribute.docs.astro.build/docs-for-code-changes/changesets/

@tmimmanuel
Copy link
Copy Markdown
Contributor Author

@matthewp
I added a changeset and CI tests are passed. Please review again. Thanks.

@tmimmanuel
Copy link
Copy Markdown
Contributor Author

@matthewp
I added a changeset and CI tests are passed. Can you review again please?

await init;
const [imports] = parse(code);

// Filter to relative JS imports only
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why filter to only relative imports?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think Vite only generates relative paths for inter-chunk references in bundled output. But please let me know if there's a case I might be missing.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah yeah that's right, ok this is good.

@matthewp matthewp merged commit de669f0 into withastro:main Mar 31, 2026
27 checks passed
@tmimmanuel
Copy link
Copy Markdown
Contributor Author

@matthewp Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg: astro Related to the core `astro` package (scope)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Skew protection assetQueryParams not applied to hoisted scripts and inter-chunk JS imports (Vercel)

2 participants