Skip to content
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

use relative asset paths #4250

Merged
merged 25 commits into from Aug 17, 2022
Merged

use relative asset paths #4250

merged 25 commits into from Aug 17, 2022

Conversation

Rich-Harris
Copy link
Member

@Rich-Harris Rich-Harris commented Mar 6, 2022

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 vitejs/vite#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:

  • 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

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

@changeset-bot
Copy link

changeset-bot bot commented Mar 6, 2022

🦋 Changeset detected

Latest commit: 90193bd

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

@iacore
Copy link

iacore commented Apr 25, 2022

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
Copy link

iacore commented Apr 25, 2022

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 vitejs/vite#2024.

Copy link
Member

@benmccann benmccann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there anything special we need to do for https://kit.svelte.dev/docs/modules#$app-paths-assets?

Assuming not, this looks good to me

@Rich-Harris
Copy link
Member Author

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

@stalkerg stalkerg mentioned this pull request Aug 16, 2022
@Amerlander
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
paths.base bugs relating to `config.kit.paths.base`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants