sapper icon indicating copy to clipboard operation
sapper copied to clipboard

Update <base> to reflect current route?

Open Rich-Harris opened this issue 7 years ago โ€ข 15 comments
trafficstars

At present, <base> is the same for all pages, which means you can't use relative URLs. This was introduced for good reasons, but it turns out to cause problems.

I'm not sure that it can be removed without breaking base URL functionality though โ€” for example, things like this would break:

<link rel=stylesheet href=global.css >
.thing {
  background-image: url(images/whatever.jpg);
}

Also, this.fetch(relativeUrl, ..) is tricky, because it needs to be relative to the page you're going to, not the page you're currently on, so it can't just straightforwardly delegate to fetch as it currently does.

It might be that the solution is just to not use relative URLs ๐Ÿ˜ฌ Unless there's already a creative solution for this problem?

Rich-Harris avatar Apr 03 '18 20:04 Rich-Harris

I would far prefer <base> removed completely and just have standard relative URLs. One big reason is same page links, e.g. <a href="#content">Skip to content</a>, currently redirect you to the index rather than just moving the viewport to the matching id="content".

As for referencing assets etc. why not do what we've always done and use a path relative to the site root:

<link rel=stylesheet href=/global.css>

Is there something with the routing that requires <base> to be set?

maxmilton avatar May 08 '18 04:05 maxmilton

In my experience <base> usually adds more confusion than it solves. Could it use just root relative urls where it's needed, for stylesheet references and the like, they can be easily prefixed with the basepath when required. Then everything else can be relative. Just my two cents. I had a quick look at the routing in Sapper and it was too complex for me ๐Ÿ˜

aubergene avatar Aug 07 '18 13:08 aubergene

Is there something with the routing that requires <base> to be set?

It's all to do with basepath. It's not uncommon to need to mount an app to a path other than root, and this was the easiest way I could find to make that possible. It is a nuisance though (for example, the other day I found that Safari and Firefox trip over url(#thing) inside SVGs, when they refer to local defs) so I'm definitely open to alternatives

Rich-Harris avatar Sep 03 '18 00:09 Rich-Harris

Why not substitute links during parsing to make it absolute? Eg

base_path = /foo/
app_path = /bar/
link: baz => /foo/bar/baz
link: /spam/ => /foo/spam

It won't solve fetch issue of course, I don't know if someone uses relative urls there. Maybe pass $path variable into each page for such case?

imbolc avatar Oct 25 '18 19:10 imbolc

Any updates on this? I'm still having issues with same page links as described by @MaxMilton

kamiyaa avatar Apr 16 '20 03:04 kamiyaa

I would solve it with #984, perhaps someone can review? It's backwards compatible so should be safe to merge in a minor version.

thgh avatar Apr 16 '20 14:04 thgh

Also solved by #1152. A workaround while waiting for a solution in sapper is monkey-patching res.end:

// add this middleware before sapper.middleware
function removeBaseTag(req, res, next) {
    const resEnd = res.end;

    res.end = function () {
        const body = arguments[0];
        if (typeof body === "string") {
            arguments[0] = body.replace(/<base[^>]+>/, '');
        }
        resEnd.call(this, ...arguments);
    }

    next();
}

arve0 avatar Apr 16 '20 18:04 arve0

Also solved by #1152. A workaround while waiting for a solution in sapper is monkey-patching res.end:

Interesting approach, but that would break routes with a slash in it because styles are loaded from client/... instead of /client/... I wish both PRs would get merged ^^

thgh avatar Apr 17 '20 07:04 thgh

but that would break routes with a slash in it because styles are loaded from client/... instead of /client/...

Not sure if you mean this particular example or the pull request, but both could make links starting with / work. The example was made intentionally minimal, to show the technique.

If it is not clear, you would use the req-object to find the correct base/link-prefix for the current request, see req.path, etc. Then replace href/src-links (/href="\/[^"]"/). To avoid replacing links that already have correct base, check if start of link is equal to the base.

This is the naive approach, and might have corner cases I have not thought out, but I believe /-links should be straight forward. Links inside javascript that is dependent on concatenation is one corner case.

I wish both PRs would get merged ^^

A base true/false-setting would be easier than replacing. Replacements on the other hand fits more needs, as it's flexible. Until it gets decided, both, one or none, I'm comfortable to patch sapper with patch-package to have the proposed replacement API ๐Ÿ™‚

arve0 avatar Apr 17 '20 10:04 arve0

Made a patch to save y'all trouble โ€“ based on @thgh's changes and can be installed with patch-package, like @arve0 suggested. Thank you guys!

notpushkin avatar Apr 23 '20 00:04 notpushkin

Are there any thoughts from people pushing for this (or the various related issues/PRs) about what hrefs should be relative to? Sapper currently allows page routes to be accessed with or without a trailing slash, which complicates the idea of relative links, and makes the whole thing messier. Even if we decided to normalize the URLs in some way (at least for the purpose of <base href> and resolving links, if not redirecting the actual pages), neither way of doing this (with or without trailing slash) is completely satisfactory, as each is more convenient in certain situations.

Conduitry avatar May 01 '20 23:05 Conduitry

@Conduitry How about /foo.svelte โ†’ /foo but /bar/index.svelte โ†’ /bar/?

notpushkin avatar May 02 '20 00:05 notpushkin

neither way of doing this (with or without trailing slash) is completely satisfactory, as each is more convenient in certain situations

I won't call having two urls pointing to the same page a perfect solution either. The classic way is having a setting for normalization behaviour on application level. The approach suggested by @nolanlawson is more flexible, but in most cases I'd prefer the normalization to be uniform within an app.

Why can't we have all the options as an url_normalization setting:

  • default - for the current behaviour
  • add_slash - redirect /foo -> /foo/
  • remove_slash - redirect /foo/ -> /foo
  • notpushkin - /foo.svelte โ†’ /foo but /bar/index.svelte โ†’ /bar/

imbolc avatar May 02 '20 03:05 imbolc

@Conduitry How about /foo.svelte โ†’ /foo but /bar/index.svelte โ†’ /bar/?

I'd be happy with this as a fix to #519. It's not about redirecting requests, that is the role of the web-server, it's about writing link hrefs to be going to the correct url in the first place

aubergene avatar May 02 '20 12:05 aubergene

I'd just like to add my ยข2: Having relative URL's will also greatly help with an internationalized site.

e.g. on a link like <a href="about"> on a page with path: /nl/ should take the user to /nl/about If links are not relative this means that you'll need to have the language in every single link.

Which is also a problem because for the purpose of DRY programming /about and /nl/about should be served by the same .svelte file and you would like to prevent moving the links into the translations files as much as possible.

Of course, the site developer will then need to make sure that path's are used correctly.

dhrp avatar Mar 04 '21 09:03 dhrp