Skip to content

Conversation

@Lofty-Brambles
Copy link
Contributor

Updates the originPathname setter to respect the trailing slash configuration. Fixes #13695

Changes

  • Adds simple handling logic to respect trailingSlash configuration to the setter.
  • Rewrites the setter's implementations to accept the configuration options necessary, from the pipelines surrounding it.

Testing

This was tested by manually reproducing it in an environment similar to the MRE mentioned in the issue.

  • An Astro project was spun up, and the local version of the package was linked.
  • index.astro and my-path.astro were added and built.
  • The dist had updated the originPathname to have a trailing slash.

On another note, should this also have unit tests? If so, can anyone guide me on where and what to add?

Docs

Since it is a simple internal fix, no user-facing documentation is necessary.

@changeset-bot
Copy link

changeset-bot bot commented May 27, 2025

🦋 Changeset detected

Latest commit: 8537607

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 May 27, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented May 27, 2025

CodSpeed Performance Report

Merging #13860 will not alter performance

Comparing Lofty-Brambles:fix/origin-path-trailing-slash (8537607) with main (442b841)

Summary

✅ 6 untouched benchmarks

@Lofty-Brambles
Copy link
Contributor Author

That's odd. Apparently pipeline manifest doesn't contain the config all the time? Build checks out, but this unit test fails. Any help on where to get this config then?

@florian-lefebvre
Copy link
Member

I've pushed a fix for the unit test

Copy link
Member

@florian-lefebvre florian-lefebvre left a comment

Choose a reason for hiding this comment

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

This needs some tests. AFAIK last time a fix about originPathname was added, a test was added in a fixture (see #13669). Maybe in this case you could create a new unit test file under test/routing?

@ematipico ematipico closed this Jun 23, 2025
@ematipico
Copy link
Member

Fixed in #13997

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.

Astro.originPathname doesn't respect trailingSlash config during build

3 participants