Skip to content

Fixes assetQueryParams and public folder tests #14853

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.

Sign up for GitHub

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

Jump to bottom
Merged
matthewp merged 8 commits into from
Nov 21, 2025
Merged

Fixes assetQueryParams and public folder tests #14853

matthewp merged 8 commits into from
Nov 21, 2025

Conversation

@matthewp
Copy link
Contributor

@matthewp matthewp commented Nov 21, 2025 •
edited
Loading

Changes

This fixes 2 different issues:

  • We weren't appending the assetQueryParam (used for skew protection) to the assets in the build.
  • Makes it so that we copy the public/ folder during the client build.
  • To make the above work, we need to always have a client build, but some times the user has no client code, in which case we use an empty noop module.

Testing

  • Fixes packages/astro/test/asset-query-params.test.js
  • Fixes packages/astro/test/static-build.test.js
  • Fixes packages/astro/test/params.test.js
  • Fixes packages/astro/test/astro-pageDirectoryUrl.test.js

Docs

N/A

@matthewp
Remove incompatible test
ca24f83 
Encoding now works the same in static and ssr mode. Since this is not
supported in SSR its also not done so in static.
@matthewp
Fixes asset-query-params.test.js
78d353b
@matthewp
Fixes the static-build.test.ts tests
36cfa18
@changeset-bot
Copy link

changeset-bot bot commented Nov 21, 2025 •
edited
Loading

⚠️ No Changeset found

Latest commit: b76bfd6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

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

matthewp
matthewp commented Nov 21, 2025
View reviewed changes
packages/astro/test/params.test.js
assert.equal($('.category').text(), '%3Fsomething');
});

it("It doesn't encode/decode URI characters such as %25 (%)", async () => {
Copy link
Contributor Author

@matthewp matthewp Nov 21, 2025

Choose a reason for hiding this comment

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

This is removed because we can no longer support this. The reason this worked before was we used the result of getStaticPaths during static builds directly. Now we produce paths and send the requests through app.render(request), so the params are in the URL. Those params get decoded for security reasons which we fixed in GHSA-hr2q-hp5q-x767

These are not encoded in SSR so they no longer can be in static.

Copy link
Member

@ematipico ematipico Nov 21, 2025

Choose a reason for hiding this comment

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

This requires a major changeset, with a thorough explanation of why we can't support this anymore. Worth adding a ticket for that

Copy link
Contributor Author

@matthewp matthewp Nov 21, 2025

Choose a reason for hiding this comment

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

Added an issue to the board

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Nov 21, 2025
@matthewp matthewp mentioned this pull request Nov 21, 2025
Merged
ematipico
ematipico reviewed Nov 21, 2025
View reviewed changes
packages/astro/test/params.test.js
assert.equal($('.category').text(), '%3Fsomething');
});

it("It doesn't encode/decode URI characters such as %25 (%)", async () => {
Copy link
Member

@ematipico ematipico Nov 21, 2025

Choose a reason for hiding this comment

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

This requires a major changeset, with a thorough explanation of why we can't support this anymore. Worth adding a ticket for that

@matthewp matthewp marked this pull request as ready for review November 21, 2025 15:33
@matthewp matthewp merged commit a947086 into feat/environment-api Nov 21, 2025
9 of 21 checks passed
@matthewp matthewp deleted the tests/env-various-tests branch November 21, 2025 15:49
ematipico added a commit that referenced this pull request Dec 4, 2025
@ematipico @penalosa
@alexanderniebuhr @ascorbic @matthewp @florian-lefebvre @sarah11918 @Princesseuh @hasegawa-101
Environment API (#14306)
141c4a2 
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ematipico ematipico ematipico approved these changes

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@matthewp @ematipico