Skip to content
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

Make trailingSlash: 'always' default #11922

Closed
wants to merge 5 commits into from
Closed

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Sep 5, 2024

Changes

There's a lot of work needed to update the test, but creating a stop gap here as something we could discuss before doing more work.

The reason for this change is highlighted in the roadmap link above. For static sites, this should not be a breaking change. For SSR sites, this will be a breaking change for on-demand pages which will now have a trailing slash by default. But you can revert to the previous behaviour for now if you want to not deal with this for now.

Testing

Docs

Copy link

changeset-bot bot commented Sep 5, 2024

⚠️ No Changeset found

Latest commit: 9440cb4

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

@github-actions github-actions bot added pkg: astro Related to the core `astro` package (scope) docs pr labels Sep 5, 2024
@matthewp
Copy link
Contributor

matthewp commented Sep 5, 2024

I'm not sure that Astro should have an opinion on trailingSlash vs not, this is mostly an aesthetic preference. I'm worried even having such a default might turn some people away from using Astro.

From the RFC discussion, it seems the problem this is trying to fix is with build.format of file, could the trailingSlash default to 'never' with that format? I could even see disallowing 'always' with file since that should be impossible to make work.

@bluwy
Copy link
Member Author

bluwy commented Sep 5, 2024

Ignoring trailing slash has an impact on SEO and reducing potential duplicate work, so I don't think it's only an aesthetic preference. For example, Next and Nuxt have trailingslash as false/'never' by default. (AFAIA only sveltekit is ignore like us). We can't go with trailingSlash: 'never' like Next and Nuxt because we already have build.format: 'directory' by default, which will cause more conflicts (and directory feels like an Astro opinion to me too 😄)

For SSG, we technically already have trailingSlash: 'always' implied by default because of build.format: 'directory'. Our sites have trailing slashes by default: https://docs.astro.build/en/basics/project-structure/ and https://astro.build/blog/images/

I don't think the RFC discussion is only about build.format: 'file', it's for both file and directory. You need them to match this way to prevent relative path issues:

build.format trailingSlash
file never
directory always

If you build your file to dist/foo/index.html, you cannot access it with example.com/foo (no trailing slash, which 'ignore' by default allows).

@matthewp
Copy link
Contributor

matthewp commented Sep 5, 2024

Going to take the discussion over to the RFC thread so that this can focus on code review.

Copy link
Contributor

@matthewp matthewp left a comment

Choose a reason for hiding this comment

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

blocked while the change is being debated

@bluwy
Copy link
Member Author

bluwy commented Sep 25, 2024

The changes here are outdated

@bluwy bluwy closed this Sep 25, 2024
@bluwy bluwy deleted the plt-1768-trailing-slash-always branch October 8, 2024 06:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs pr pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants