-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
trailing slash removed when navigating #733
Comments
The adapater repo is here : https://github.com/wighawag/sveltejs-adapater-ipfs The test app is here : (branch : adpater-ipfs) can be found here : https://github.com/wighawag/svelte-kit-ipfs-example/tree/adapter-ipfs With the current modif it works but ideally I would not need to replace generated code as this is brittle |
previous discussion here, which lead to #267. from there:
Sounds like we have a use case! What we don't have is a sense of how this should be configured. Is an app-wide setting sufficient, or does it need to be per-page? What sort of thing did you have in mind? |
Hi @Rich-Harris thanks for the link to past discussion
For my use case (ipfs hosting), an app-wide config would be sufficient ipfs gateway force the trailing slash when an index.html is detected and used. The trailing slash make it easier to compute relative path too |
Another use case is SEO, the same route with slash and without would be indexed as two different pages by a crawlbot, therefore deciding to redirect to either or makes sense. |
FWIW regarding relative paths, I borrowed/tweaked this url building from routify's helpers to better handle them import { derived, get } from 'svelte/store';
import { page } from '$app/stores';
import { goto as gotoApp } from '$app/navigation';
export function url(path: string) {
const $page = get(page);
if (path.match(/^\.\.?\//)) {
// Relative path (starts with `./` or `../`)
let [, breadcrumbs, relativePath] = path.match(/^([\.\/]+)(.*)/);
let dir = $page.path.replace(/\/$/, '');
const traverse = breadcrumbs.match(/\.\.\//g) || [];
// if this is a page, we want to traverse one step back to its folder
traverse.forEach(() => (dir = dir.replace(/\/[^\/]+\/?$/, '')));
path = `${dir}/${relativePath}`.replace(/\/$/, '');
path = path || '/'; // empty means root
console.groupEnd();
} else if (path.match(/^\//)) {
// Absolute path (starts with `/`)
return path;
} else {
// Unknown
return path;
}
return path;
}
export function goto(path: string) {
const newPath = url(path);
return gotoApp(newPath);
} Examples: <a href={url(`./${item.Id}`)}>Child / Subitem path</a>
<a href={url(`../${key}`)}>Sibling path</a>
<a href={url('../')}>Parent path (back to list, etc)</a>
<a href={url('../../')}>Grandparent path</a> |
What's wrong with just using relative paths? |
@Rich-Harris it's been a little while to remember all the details, but when I tried to use For example, if I want to add a child/sub path (tack on a new URL('id', 'http://example.com/one/two/three').pathname
=> "/one/two/id" new URL('./id', 'http://example.com/one/two/three').pathname
=> "/one/two/id" I think this is mostly due to the path not ending in a There are more use cases I have such as those listed above (replace the current last path (sibling path), parent path, etc). |
Another use case to to not remove the trailing spaces, is how Netlify handles URLs by default. URLs without a trailing spaces will automatically get redirected (301) to the one with a trailing slash, as you can see in the below screenshot. This is impacting performance a little bit, which I measured with web.dev. You can change it by updating the asset deployment settings of Netlify (disable the prettifier for URLs), but it is not clear this is the solution to this issue, and it (potentially) impacts other URLs in a negative way. FYI, I am using the static adapter to deploy to Netlify. |
Netlify redirects tries the exact path in the request and if that is not available it will redirect to trailing slash or non-trailing slash whichever one is available. |
I've been looking into SvelteKit routing deeper recently for #943.
Because there's no other way to scope those two features down to a specific path if we're using filesystem based routing. The trailing slash and non-trailing slash URLs can exist at the same time and support existing features in a couple of different scenarios. E.g.
The problem with this approach is that
This might be a little confusing at first glance but allows scoping of layout and hooks.
I think this probably the cleanest. We could also go with I think any of these can be implemented without breaking existing behavior. I'm willing to put in the work and implement this since we have a need for this kind of routing for a website we want to port to SlvelteKit. But I'd like to get a go ahead from SvelteKit authors and maintainers first. |
@RaeesBhatti It is an interesting approach to allow for both. I still think we need a way to have that configurable globally though and let user use normal folder name if desired. This is especially important for ipfs gateway which do not allow for path that do not ends with slash. They get redirected to path with a trailing slash. It would be confusing to force user to use Your proposal should be an opt-in option that let users use both trailing slash and non-trailing slash if they need it, so that use of trailing slash can continue to use the normal folder name. |
@Conduitry could you chime in with your opinion on this! I personally prefer option 3. It can be documented in the routing section to quell any confusion. |
Another vote and use case for not stripping trailing slashes: write a blog inside of SvelteKit, use MDsvex for posts, and attempt to link to a resource in the same directory. Write the post |
This may be a more serious issue. It turns out that Netlify redirects to the trailing slash version and SvelteKit redirects to the version without trailing slash so you get an infinite redirect: #1346 |
Is it connected to the Pretty URLs feature discussed in the netfify guide? |
I actually have all the optimizations by Netlify disabled, so I would say not related to that. |
Does SvelteKit really have to care about slashes at all (sapper doesn't)? This is a blocker for us, cause our site has been in production for 2 years. We don't want to change all canonical paths, sitemaps, and reindex all search engines. We're using a hybrid atm,
This should probably be mentioned at the top of the migration guide. |
@bjon Sapper has an open issue of sveltejs/sapper#519 and it states not that it does not care, but that it inconsistent and it is confirmed at the end with multiple 👍🏻 reactions that this behavior in fact affects people. Speaking for myself, I would love to have it in kit consistently without a trailing slash by default, with the possibility of an individual override. |
I don't really care how its done, as long as its possible to migrate from sapper :) |
To me the most straightforward way to do the routing seems to be the following:
I.e. the last |
I've gone back and forth on @bdadam's suggestion — there is a pleasing simplicity to it, but I don't love the fact that path semantics are dictated by what are essentially code organisation decisions. An alternative would be to add the trailing slash if and only if there's a non-index page in the directory as well — i.e. There's another wrinkle, which is that the index-ness of a URL can't actually be determined immediately, in the general case. Consider a scenario in which you have a Then again, those cases would be rare, so we might be willing to live with that failure case. One possibility that should cover all the bases would be to have an app-wide
Thoughts? We could also start with just |
@Rich-Harris that would work great for us |
I agree with having The |
But that's true for the entirety of file based routing. I would love to have login, registration, etc. in their own folder named @bdadam's suggestion is not only easy to teach and implement, but it is also the only one that is deterministic. Under that scheme, I can look at the current URL and tell you exactly which file path to check for the entry point. For every other proposed scheme, I know the general ballpark to look but I will have to look at the filesystem to determine exactly which file to open. It does also allow both
The argument for For internal links in a project, |
Interesting — I would have regarded that as a bug, not a feature. The fact that they're technically separate URLs has always felt like an accident of history; I can't imagine a situation in which you'd want to have
This is true (up to a point — I don't know that Also, it introduces a wrinkle with prerendering: at present,
The benefit isn't to the user, it's to the developer. It means that your <a href="./{post.slug}">{post.title}</a> ...rather than this (which is less portable — you can't just rename the directory 'articles' or trivially handle localised routes): <a href="/blog/{post.slug}">{post.title}</a> |
Opened a separate issue to discuss |
In case someone else also has to figure this out: |
file browsing in browser (for example for cloud storages or repos, or torrent trackers),... if it's ending with symbol |
But in that case you're not going to have a new |
@Rich-Harris Thanks for implementing I could imagine either exporting it in trailingSlash: (path) => true OR <script context="module">
export const trailingSlash = true;
export async function load({ page, fetch }) {
return {
props: {
// ...
},
};
}
</script> What do you think? |
@bdadam is it entirely necessary? I believe having an inconsistent behavior here here is an anti-pattern. Also with Although SvelteKit is aiming to support services like Vercel or Netlify, where the webserver config is not available, there are probably many other uses for the Kit that might not even involve aforementioned services (for instance waiting to be able to finally use things like https://github.com/svelte-add/tailwindcss in production env), but maybe I am wrong here. |
Agree, if we are talking about |
You can still do that today in SvelteKit it you'd like |
I just filed an issue in the static adapter which IMHO would be easier to tackle if the file path determined what the output filename is, e.g. |
FYI: relative paths within a file like /xyz/index.svelte pointing to /subfolder/123 do not translate to /xyz/subfolder/123 for me (using ordinary links). very weird and not what i was expecting. #733 (comment) solution did however solve it. just leaving this here as i am not sure if this is a bug or just how svelte works. |
Use case: Hosting on AWS S3. |
Describe the bug
I would like my path to end up with a trailing slash.
Currently with these lines, svelte kit always remove it :
kit/packages/kit/src/runtime/client/router.js
Lines 185 to 188 in 84e9023
Expected behavior
This behavior should at least be configurable.
Having a trailing slash is important to me as ipfs gateways append it anyway and so without being able to keep the trailing slash, there is a different behaviour between SPA navigation and navigation through request.
Plus it cause issue to calculate relative path as without slash the resource is now considered to be in the parent folder
I think trailing slash are more accurate in that regard since on a static website, the corresponding underlying index.html file is in its own folder.
Severity
This is a blocker for me that deploy website on ipfs
I am working on a ipfs adapter to support ipfs properly and I am currently stuck with this.
The text was updated successfully, but these errors were encountered: