Fix $app/paths import not working at dev mode inside context="module"

[istarkov]

ref #2108

I've started playing with #2108 and found that

<script context="module">
import { base, assets } from '$app/paths';
...

Always return default values at dev and preview at serverside.

This is attempt to fix above.

Steps.

• add failing test (created standalone for now as setting paths.base at options etc breaks everything)
• add fix

Without above its hard to proceed with full paths.base support. As { base } import at module level would be needed for tests.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 7cb7845

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[istarkov]

ps: locally atop of this PR, I have fix for #2108 in dev and preview modes

In short I changed manifest routes pattern generation to include basePath, and cleared few places where basePath were spliced from pathname.

Even such solution doesnt allow dynamic basePath now, but it can be extended to be dynamic by changing pattern match logic a little bit.

[benmccann]

Is this something you're going to want to do for every URL within the app or should we add a base html tag or something?

[istarkov]

btw I havent knew about base tag. :-) so probably its much better.

Would check tomorrow

[Rich-Harris]

Thank you — the current behaviour was semi-intentional, but a little bit confusing (and error-prone, since you could easily forget to prefix URLs with base or assets), and this is a good opportunity to clarify it.

I think the base stuff makes sense. For assets it's a little trickier — a common use case for this option is to have your static assets hosted on a CDN far from your actual app, and as such it doesn't really make sense in dev or preview (since up-to-date assets won't have been uploaded to the CDN, and since it prevents things like offline development).

So I wonder if it makes sense to have a fake assets path during dev and preview? I wrote this up in #326 but then never got round to doing anything about it. We'd probably need to avoid changing req.url, since IIUC static_handler would respond to requests for /photo.jpg as well as ${assets}/photo.jpg, which we want to avoid.

[istarkov]

Probably one of options would be to simplify a little bit.

In case of relative assets what the use case for asset to be not equal to base ?

For example our use case is PR deployments, because of 3rd party lib referer restrictions (no wildcards supported etc) we can't deploy each PR on own sub domain. So what we really need that same code work on any base path depending on env variable.

With our usecase we can set <base href="//{host}/{base}"> (thanks @benmccann I totally forgot about it) and everything like <a href='/blabla' or <img src="/blabla" would work.

If someone need different path for assets seems like changing appDir or/and adding subfolder into static will be enough.
As have different assets dir for different base makes code much more complicated.

So probably assets config must accept only non relative urls with schema defined i.e https://blabla, in all other cases assets must be same as base url.

At least for our usecase no need to do something like <img src="{asset}/blabla"/> and no need in fake path (as base tag would be enough).

In case of non relative assets fake path can be an option.

[Rich-Harris]

In case of relative assets what the use case for asset to be not equal to base ?

🤯 this is... a great point. I don't think there is a good use case. We should probably just prohibit relative asset paths and use a fake one during development and preview. I have a bit of time today so will work on this now.

I've found <base> to be a pain in the neck, so in the long run I'd love for us to support dynamic basepaths. There's an issue for that — #595 — but it can be addressed separately from this

[Rich-Harris]

closing in favour of #2189 — thanks