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 a page/endpoint option, like prerender #7719

Merged
merged 10 commits into from
Nov 21, 2022
Merged

Conversation

Rich-Harris
Copy link
Member

@Rich-Harris Rich-Harris commented Nov 18, 2022

How to migrate

  1. Remove the option from from svelte.config.js
  2. Add the option to src/routes/+layout.js:
export const trailingSlash = 'never'; // or 'always' or 'ignore', depending on what you want

PR description

Closes #7302. This moves trailingSlash configuration from svelte.config.js and into pages. Benefits:

  • Moving stuff out of config and into code is always good
  • We can now have e.g. /blog/ and /blog/my-post
  • We can now control trailing slashes on endpoints (niche, but will probably be useful one day)

It has very limited impact on client-side bundle size or data transfer.

The downside is that on the server, we need to load code for the route before we can return a redirect response. In practice the impact is negligible (since we don't need to render anything, just import/evaluate it) and in any case it's not the most important thing to optimise.

TODO:

  • documentation
  • delete packages/kit/test/prerendering/trailing-slash etc — no longer necessary, the tests can be moved into other apps

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

@Rich-Harris Rich-Harris changed the title Gh 7302 Make trailingSlash a page/endpoint option, like prerender Nov 18, 2022
@changeset-bot
Copy link

changeset-bot bot commented Nov 18, 2022

🦋 Changeset detected

Latest commit: 0b5da1b

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

This PR includes changesets to release 2 packages
Name Type
@sveltejs/adapter-vercel Patch
@sveltejs/kit Patch

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

Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

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

Looks good code-wise. Question I have though: How likely is it that people are pissed off by this because they now have to add export const trailingSlash = ".." to all of their +server.js files? Previously, you had it to set it only once in the config, and it applied to endpoints as well.

@Rich-Harris
Copy link
Member Author

Previously, you had it to set it only once in the config, and it applied to endpoints as well.

Not quite — trailing slashes were completely ignored for endpoints. Now they'll be treated as never by default, which I think is what people will want in the overwhelming majority of cases (it's only really pages where a trailing slash ever makes sense), but in the exceptional cases where they do want to enforce a trailing slash they can now do so much more easily than before, where they had to implement the redirect logic themselves

@Rich-Harris Rich-Harris merged commit f87d55b into master Nov 21, 2022
@Rich-Harris Rich-Harris deleted the gh-7302 branch November 21, 2022 16:06
FSMaxB added a commit to communityvi/communityvi that referenced this pull request Dec 4, 2022
brettski added a commit to ThatConference/thatconference.com that referenced this pull request Mar 16, 2023
brettski added a commit to ThatConference/thatconference.com that referenced this pull request Mar 16, 2023
brettski added a commit to ThatConference/thatconference.com that referenced this pull request Mar 16, 2023
* feat: semver management update

* fix: update svelte for vercel adapter dependency

* fix: vercel adapter needs to be pre-installed

* fix: trailingHash alaways handled differently in sveltekit v1
sveltejs/kit#7719

* fix: update all +server.js files for trailing slash
sveltejs/kit#7719

* fix: remove trailing slash from proxy endpoint

* fix: vercel deployment

* fix: rollback: re-enabling proxy trailing slash

* chore: add depth to sentry log

* chore: added proxy debugging

* chore: refined sentry

* chore: refined sentry

* fix: refine logging for auth testing

* chore: logging adjustments

* chore: different logging, auth proxy issue

* chore: console res body in proxy

* chore: console res body in proxy

* fix: correct auth issues, remove console statements, turn off bot mode at CloudFlare on api
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

trailingSlash override functionality
2 participants