kit icon indicating copy to clipboard operation
kit copied to clipboard

use relative asset paths

Open Rich-Harris opened this issue 2 years ago • 11 comments

This was just a quick experiment inspired by #3014 to see if it's possible to create prerendered SvelteKit apps that don't even need a webserver.

It basically works — if you have routes like src/routes/index.html.svelte instead of index.svelte, you can double-click on the resulting build/index.html file, and you have an (almost) fully working site, because the generated CSS and JS are loaded from file:// URLs. Might be useful in certain contexts, and is a step towards #595.

One thing it doesn't account for is fallback routes (i.e. if you visit /foo/bar/baz in an SPA and it's served by the fallback page, the relative URLs will be wrong). The biggest issue though is that Vite uses absolute URLs — if you import an asset from JS or CSS, it will be imported from an absolute URL rather than a relative one. I think that could be changed (along with https://github.com/vitejs/vite/issues/2009) for the benefit of all, as it would make every Vite app more portable. Unless and until that happens, I'm going to mark this as draft and not spend any more time on it.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • [x] 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
  • [x] This message body should clearly illustrate what problems it solves.
  • [x] Ideally, include a test that fails without this PR but passes with it.

Tests

  • [x] Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • [ ] If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpx changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

Rich-Harris avatar Mar 06 '22 16:03 Rich-Harris

🦋 Changeset detected

Latest commit: 90193bdaa8c2cff1d2c4f79bc4d2ebf5613c4820

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sveltejs/kit Patch

Not sure what this means? Click here to learn what changesets are.

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

changeset-bot[bot] avatar Mar 06 '22 16:03 changeset-bot[bot]

I've kept this up to date with master branch. Please use it when the time comes to merge this. https://github.com/locriacyber/kit/pull/new/relative-assets

https://github.com/jy0529/vite-plugin-dynamic-publicpath is out now. Maybe use it to resolve dynamic import?

iacore avatar Apr 25 '22 17:04 iacore

I was able to track it down to .svelte-kit/output/client/_app/chunks/preload-helper-e4860ae8.js. Content of the file:

const a = "modulepreload",
  o = {},
  u = "/_app/",
  h = function (s, n) {
    return !n || n.length === 0
      ? s()
      : Promise.all(
          n.map((e) => {
            if (((e = `${u}${e}`), e in o)) return
            o[e] = !0
            const t = e.endsWith(".css"),
              i = t ? '[rel="stylesheet"]' : ""
            if (document.querySelector(`link[href="${e}"]${i}`)) return
            const r = document.createElement("link")
            if (
              ((r.rel = t ? "stylesheet" : a),
              t || ((r.as = "script"), (r.crossOrigin = "")),
              (r.href = e),
              document.head.appendChild(r),
              t)
            )
              return new Promise((l, c) => {
                r.addEventListener("load", l),
                  r.addEventListener("error", () =>
                    c(new Error(`Unable to preload CSS for ${e}`))
                  )
              })
          })
        ).then(() => s())
  }
export { h as _ }

Note that "/_app/" was here.

If there is a way to change the way import(xxxxx) is compiled, this could be solved.

This was mentioned in https://github.com/vitejs/vite/issues/2024.

iacore avatar Apr 25 '22 18:04 iacore

Ok I think this is ready. The one caveat is that we need to use the user-specified base for the SSR build (rather than ./) because otherwise imported assets will have the wrong URL, which results in things like images 404ing for a split second before hydration kicks in.

Nothing really changes as a result of this PR (other than apps being more resilient to things like finding themselves on the internet archive, etc), but it hopefully paves the way for #595 etc.

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

Ok I think this is ready

It's not ready. A weird thing I noticed is that since this change, workers are built to the wrong place. They're already built to the wrong place, but now they're built to a wronger place. If you visit https://kit.svelte.dev/ and inspect the network tab, you'll see this file:

https://kit.svelte.dev/_app/assets/search.0db017ff.js

That's the web worker that powers the search function. It should be in _app/immutable but for some reason it's not, it's just in _app, which means it won't get properly cached.

As of this PR, it's not even inside _app — it's built to /assets/search.0db017ff.js. I've tried setting worker.rollupOptions.output but it doesn't seem to be having any effect. Still investigating.

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

I believe I've found two bugs in Vite:

  1. worker.rollupOptions.output is not respected if it's set inside a plugin's config hook

  2. When building with base: './', workers are instantiated with new Worker('../../path/to/worker.js'), rather than with a root-relative path beginning with base. But that's wrong — the path will be relative to the document, not the module instantiating the worker. I think it should be something like this:

    new Worker(new URL('../../path/to/worker.js', import.meta.url).href)
    

Will see if I can cobble together a repro later, but cc'ing @bluwy for visibility in the meantime

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

Ok I think this is ready. The one caveat is that we need to use the user-specified base for the SSR build (rather than ./) because otherwise imported assets will have the wrong URL, which results in things like images 404ing for a split second before hydration kicks in.

We hit similar problems when Nuxt started to experiment with relative base. They are now using base: './' together with a new experimental feature that lets you have fine-grain control over the base for each asset type, conditional on where are they imported and if it is an SSR run. It is experimental but we don't plan to change this as Nuxt 3 will be using it (so if we change the API it will be in Vite 4 in a few months once we move it out of experimental)

There is a new experimental.renderBuiltUrl option that takes a function with type:

export type RenderBuiltAssetUrl = (
  filename: string,
  type: {
    type: 'asset' | 'public'
    hostId: string
    hostType: 'js' | 'css' | 'html'
    ssr: boolean
  }
) => string | { relative?: boolean; runtime?: string } | undefined

In SvelteKit, I think you have your own implementation of public files and handling of HTML, so you may need to replicate some of the handlings we did for them. So you can use it to do something different in SSR:

experimental: {
  renderBuiltUrl(filename: string, { ssr, hostType }) {
    if (ssr) {
      if (hostType === 'js') { 
        return { runtime: `globalThis.__svelteKit_SSR_toAssetPath(${filename})` }
      }
      else if( hostType === 'css' ) {
        return base + filename
      }
    }
    else {
      return { relative: true }
    }
  }
}

You can return { relative: true } for a subset of assets, or depending on SSR.

If you return a string, it will be directly replaced:

code.replace(viteAssetRE, url)

And you can use { runtime: string } (only if the hostType is 'js') to give Vite code that should be executed dynamically (like we do with new URL(..., import.meta.url) for relative base). In this case, if the asset is in a CSS string in JS, for example, Vite will interject that inside the string as:

code.replace(viteAssetRE, `" + runtime + "`)

Nuxt uses this to implement dynamic runtime base for the client with something like:

experimental: {
  renderBuiltUrl: (filename, { hostType }) => {
    if (hostType === 'js') {
      return { runtime: `window.__toCdnUrl(${JSON.stringify(filename)})` }
    } else {
      return { relative: true }
    }
  }
}

PR for reference

  • https://github.com/vitejs/vite/pull/8762

About the issues you found, I'll check them out next week. Having them as bug reports in vite to track them would help.

patak-dev avatar Jul 15 '22 19:07 patak-dev

  1. When building with base: './', workers are instantiated with new Worker('../../path/to/worker.js'), rather than with a root-relative path beginning with base. But that's wrong — the path will be relative to the document, not the module instantiating the worker. I think it should be something like this:
    new Worker(new URL('../../path/to/worker.js', import.meta.url).href)
    

This should be fixed in the next patch by:

  • https://github.com/vitejs/vite/pull/9204
  1. worker.rollupOptions.output is not respected if it's set inside a plugin's config hook

For this one, I'll wait for the repro.

patak-dev avatar Jul 18 '22 22:07 patak-dev

thank you, done! https://github.com/vitejs/vite/issues/9205

Rich-Harris avatar Jul 18 '22 23:07 Rich-Harris

@Rich-Harris both fixes were released as part of Vite 3.0.3

benmccann avatar Jul 26 '22 22:07 benmccann

I don't think so. This PR still uses an absolute base in SSR mode, as I couldn't figure out an elegant way to use renderBuiltUrl to create relative asset paths at SSR time; will leave that problem for another day. Just waiting to see if the windows failure is flakiness or not

Rich-Harris avatar Aug 01 '22 02:08 Rich-Harris

Is there an option to keep absolute paths when using static adapter? I'm not able to use vite imagetools with that: https://github.com/JonasKruckenberg/imagetools/issues/367

Amerlander avatar Aug 24 '22 23:08 Amerlander