vite icon indicating copy to clipboard operation
vite copied to clipboard

Ability to serve base page without trailing slash

Open benmccann opened this issue 3 years ago • 2 comments

Description

SvelteKit has a trailingSlash option which indicates whether we expect URLs to have a trailing slash or not. By default all URLs do not have a trailing slash. However, Vite forces a trailing slash on the base URL.

Implementing this would close https://github.com/vitejs/vite/issues/8770 and https://github.com/vitejs/vite/pull/8772

Suggested solution

Add a new trailingSlash option similar to SvelteKit's. I think it would likely only affect the base URL though for Vite

Alternative

Vite could accept base with or without a trailing slash so that the user can decide which they would prefer. However, this would be a breaking change as anything reading base from the config would now have to handle the possibility both of it having or not having a trailing slash

https://github.com/vitejs/vite/pull/8772 is open which affects this functionality as well, but would not work as a solution for SvelteKit as it always forces a trailing slash and would conflict with SvelteKit's trailingSlash option

Additional context

SvelteKit historically hasn't used the publicDir and base options, but does its own static asset serving. I put together a branch to use Vite's implementation, but some of our tests are failing with it due to this issue, which is a blocker for us.

https://github.com/sveltejs/kit/pull/5601/files#r923967104

Validations

benmccann avatar Jul 20 '22 03:07 benmccann

I don't think Vite needs a new option, it should just accept /foo as a valid pathname when base === '/foo', instead of inviting you to visit /foo/foo

Rich-Harris avatar Jul 21 '22 15:07 Rich-Harris

Yeah, I mentioned that option as well. It's a simpler user API, but likely to be a breaking change and harder for developers to use. I don't know which is better and am fine with either solution

benmccann avatar Jul 21 '22 15:07 benmccann

This was discussed at today's meeting. It was suggested to put together a PR for it. We also discussed the possibility of a helper method for handling appending a path to a base URL. We were trying to figure out how to help frameworks migrate and thought one possibility might be that the helper could be backported to 3.2 so that frameworks can start adopting the helper and when we disable enforcement of the trailing slash there will not be an effect on the frameworks that have already moved to using the helper.

benmccann avatar Oct 21 '22 21:10 benmccann

https://github.com/vitejs/vite/pull/10590 was merged in order to append the URL to the base path in a safe manner throughout the codebase regardless of whether it ends with a / or not. Assuming I didn't miss any places we were doing that then hopefully we're a good chunk of the way there.

The first thing would be to change the config validation to not enforce a trailing /:

https://github.com/vitejs/vite/blob/675bf07a093c2af5c928bdd1a8458dc235cc442d/packages/vite/src/node/config.ts#L815

Possibly the base middleware would need some small tweaks

https://github.com/vitejs/vite/blob/main/packages/vite/src/node/server/middlewares/base.ts

And we should add a test to ensure that a base URL with no trailing slash works as intended

benmccann avatar Oct 27 '22 16:10 benmccann

You missed the place where it says // prepend base (dev base is guaranteed to have ending slash) - and it took me forever to find that! ;)

Nevertheless, I've opened a PR for this and I think the basic functionally is there already. Some behavior has to be decided on and maybe someone else can help me with the remaining broken tests.

BenediktAllendorf avatar Oct 30 '22 18:10 BenediktAllendorf

That's great! Thank you so much @BenediktAllendorf! I might not be able to look at this week, but will definitely try to help get it in before the first 4.0 alpha release. Thanks for finding the place I missed! (I searched for everywhere config.base was used, but missed that one since it was destructured)

benmccann avatar Oct 31 '22 05:10 benmccann