dynamic base

[wighawag]

Implement dynamic base like assets is currently handled

this is one step more for #595
The only extra step required after this PR is in, is to fix the pre-rendered html and js file to use relative paths
See : https://github.com/bug-reproduction/svelte-kit-static-ipfs/tree/fixes#fixes and https://github.com/wighawag/sveltejs-adapter-ipfs

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
  3 test seems to fail but they do not seem to be related

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. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

[benmccann]

Thank you! We will need to get the tests in packages/adapter-static passing as it looks like they're failing with this change

[benmccann]

I wonder if there might be a simpler way than computing the base everywhere we instantiate the application. In client.js today we import the base:

kit/packages/kit/src/runtime/client/client.js

Line 28 in 75e1464

import { base } from '__sveltekit/paths';

Perhaps in the beginning of client.js we can compute the base relative to the deployment instead as Rich suggested here: #595 (comment)

[wighawag]

Is it not already doing that ?

we need it to be executed once per page load and with the current code in that PR it does exactly that by having it executed in the index.html

It uses the same technique used by assets

[benmccann]

Is it not already doing that ?

You said we still need to "fix the pre-rendered html and js file to use relative paths". I'm not entirely sure what needs to be fixed there, but was hoping the method Rich suggested would avoid solve that.

It uses the same technique used by assets

It looks like @Rich-Harris just added that two days ago in #8957. We probably should not implement it that way as it's going to break things like Storybook and Vitest. They'll import components directly without instantiating an entire application and so globals defined in the page won't be available. We should probably setup some sort of test to stop these sorts of regressions. In the meantime, I'd rather not double down on that implementation.

[wighawag]

You said we still need to "fix the pre-rendered html and js file to use relative paths". I'm not entirely sure what needs to be fixed there, but was hoping the method Rich suggested would avoid solve that.

Oh the extra step are still needed because currently that solution only set the base value (but now dynamically) that we can then get by using import {base} from `$app/paths` but it does not uses it everywhere it needs to be,

Also the extra step allow the sveltekit app to still work when no javascript is running by replacing all absolute path svelte generate into relative one

It looks like @Rich-Harris just added that two days ago in #8957. We probably should not implement it that way as it's going to break things like Storybook and Vitest. They'll import components directly without instantiating an entire application and so globals defined in the page won't be available. We should probably setup some sort of test to stop these sorts of regressions. In the meantime, I'd rather not double down on that implementation.

ok, yes once the new thing is in, we can adapt to ensure base and assets are set dynamicaly

[benmccann]

You can ignore my earlier comment about the alternate implementation in #9150 (comment). The way this PR is implemented is basically correct. For an explanation to my later self and other readers, I believe if we are serving a fallback page there is no way to have a dynamic base path because I'm not sure how the client would differentiate the part of the request path that is the base path vs the route path. If a page is prerendered or dynamically rendered then we can calculate the distance from the base at render time in order to set the base variable at runtime. That's what this PR does.

However, the \0__sveltekit/paths virtual module is now basically useless, which is still a concern I have. The relative path stuff is only really needed when prerendering as I believe that's the only case where the dynamic base path feature will be used. On the otherhand, the virtual module path support for Storybook, etc. is only used outside the context of a page and I think it's unlikely users would have a need for dynamic base path with Storybook, Vitest, etc.

I think what we should do is set the base path the traditional way in the virtual module most of the time and just use this new relative method if prerendering or rendering a fallback page. The easiest option would probably be to access base and assets from global with optional chaining and fallback to the traditional values if they're not specified and then only define base and assets in global when prerendering.

[Rich-Harris]

I think what we should do is set the base path the traditional way in the virtual module most of the time and just use this new relative method if prerendering or rendering a fallback page

An app can contain prerendered and non-prerendered pages, so that's not an option. If referencing __sveltekit_<hash> is problematic then we could tweak that code (window.__sveltekit_<hash>?.assets ?? '', etc), or better still we could generate a slightly different module when we detect something like Storybook or Vitest

[benmccann]

Oh yeah, for some reason I was thinking the virtual module was a per-request thing, but I guess it's a per-app thing. The kids got me sick for like the fifth time since Thanksgiving, so my head's not been working too well.

I feel like the ?? approach might be better as I'm not sure how we'd detect all the various use cases and I don't really want to hardcode knowledge of those tools. It is a few extra bytes for most users, but it's pretty minimal at least

[benmccann]

does this need a changeset?

otherwise, lgtm. I'll fix the virtual module thing in another PR after I figure out what's going on with #9162

[Rich-Harris]

Added hardcoded fallbacks — this should solve the Vitest case.

I think we can solve the prerendering case easily enough. In fact the pre is a red herring, as the solution applies equally to content that is rendered on demand. Made a start in #9220 — might be nice to get that in as part of this, so it's a single minor release.

[benmccann]

Implemented in #9220

[benmccann]

Thank you for driving this feature!! It wouldn't have gotten done without your efforts