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

Base hrefs are not supported in prerendering #8559

Closed
tontonsb opened this issue Jan 16, 2023 · 4 comments · Fixed by #9257
Closed

Base hrefs are not supported in prerendering #8559

tontonsb opened this issue Jan 16, 2023 · 4 comments · Fixed by #9257
Labels
bug Something isn't working low hanging fruit ready to implement please submit PRs for these issues!
Milestone

Comments

@tontonsb
Copy link

tontonsb commented Jan 16, 2023

Describe the bug

It appears that base hrefs (e.g. <base href="/en/">) are not supported at the moment. I stumbled against two issues.

One issue is that the prerenderer/crawler grabs all hrefs and resolves the relative references against the current path. The result is something like this:

  • subsection resolved from /en/section => /section/subsection
  • section resolved from /en/section/subsection => /section/section

Note that this works fine in the browser, the problem is only with the prerenderer. I tinkered around with the code and managed to fix it by resolving against base in this line and the base itself is fairly easy to obtain in the crawl() that's called above.

The second issue is that the asset paths are relative. And the base href screws up all of them as ./_app gets resolved to /en/_app in the browser. This could be solved by allowing to turn off the asset relativity. It might be as simple as allowing / in config.kit.paths.assets. Currently it is refused by a param check with the Error: config.kit.paths.assets option must be an absolute path, if specified. and Error: config.kit.paths.assets option must not end with '/'. checks.

Reproduction

https://github.com/tontonsb/sveltekit-base-prerendering

Try npm run dev and visit http://localhost:5173/en to observe that you can either follow links or directly visit /en, /en/section and /en/section/subsection.

To observe the problem with the crawler:

  1. Execute npm run build and you'll see Not found: /section/subsection
  2. OK, let's make the language prefix optional. Try git checkout lang-optional and npm run build. Now /section/subsection will work (as the [[lang=lang]] prefix is optional in this branch), but you'll stumble against Not found: /section/section when it tries to resolve section from /section/subsection.
  3. In case you want more fun, setting config.kit.prerender.handleHttpError = 'warn' will throw you into infinite 404 /section/section/section/sect... :)

To observe the problem with assets:

  1. Do git checkout assets and npm run build && npm run preview.
  2. Browse around http://localhost:4173/en.
  3. Observe the errors in your console that report things like Not found: /en/_app/immutable/start-c96f4d66.js.

Logs

Example of the crawler error when running npm run build:

Error: Not found: /section/subsection
    at resolve (file:///path/to/sveltekit-base-prerendering/.svelte-kit/output/server/index.js:2493:18)
    at resolve (file:///path/to/sveltekit-base-prerendering/.svelte-kit/output/server/index.js:2358:34)
    at Object.#options.hooks.handle (file:///path/to/sveltekit-base-prerendering/.svelte-kit/output/server/index.js:2537:59)
    at respond (file:///path/to/sveltekit-base-prerendering/.svelte-kit/output/server/index.js:2356:43)
file:///path/to/sveltekit-base-prerendering/node_modules/@sveltejs/kit/src/core/postbuild/prerender.js:27
                                throw new Error(format(details));
                                      ^

Error: 404 /section/subsection (linked from /en)
    at file:///path/to/sveltekit-base-prerendering/node_modules/@sveltejs/kit/src/core/postbuild/prerender.js:27:11
    at save (file:///path/to/sveltekit-base-prerendering/node_modules/@sveltejs/kit/src/core/postbuild/prerender.js:346:4)
    at visit (file:///path/to/sveltekit-base-prerendering/node_modules/@sveltejs/kit/src/core/postbuild/prerender.js:195:3)
[vite-plugin-sveltekit-compile] Prerendering failed with code 1
error during build:
Error: Prerendering failed with code 1
    at ChildProcess.<anonymous> (file:///path/to/sveltekit-base-prerendering/node_modules/@sveltejs/kit/src/exports/vite/index.js:599:15)
    at ChildProcess.emit (node:events:513:28)
    at Process.ChildProcess._handle.onexit (node:internal/child_process:291:12)

Example of the asset errors after npm run build && npm run preview in branch assets:

Error: Not found: /en/_app/immutable/start-c96f4d66.js
    at resolve (file:///home/juris/sveltekit-base-prerendering/.svelte-kit/output/server/index.js:2493:18)
    at resolve (file:///home/juris/sveltekit-base-prerendering/.svelte-kit/output/server/index.js:2358:34)
    at Object.#options.hooks.handle (file:///home/juris/sveltekit-base-prerendering/.svelte-kit/output/server/index.js:2537:59)
    at respond (file:///home/juris/sveltekit-base-prerendering/.svelte-kit/output/server/index.js:2356:43)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

System Info

System:
    OS: Linux 5.10 Ubuntu 22.04.1 LTS 22.04.1 LTS (Jammy Jellyfish)
    CPU: (8) x64 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz
    Memory: 4.66 GB / 7.61 GB
    Container: Yes
    Shell: 3.6.0 - /usr/bin/fish
  Binaries:
    Node: 16.17.0 - ~/.local/share/nvm/v16.17.0/bin/node
    npm: 8.15.0 - ~/.local/share/nvm/v16.17.0/bin/npm
  npmPackages:
    @sveltejs/adapter-static: ^1.0.0 => 1.0.4 
    @sveltejs/kit: ^1.0.0 => 1.1.1 
    svelte: ^3.54.0 => 3.55.1 
    vite: ^4.0.0 => 4.0.4 

Severity

serious, but I can work around it

Additional Information

It seems that localization with a path prefix is not really possible unless one is willing to manually import a lang store and add a /{$lang}/ prefix to all internal links.

While some workarounds could be done as outlined above, the best solution would be official support for path prefixes. Be it with base href or other method of prepending the prefix to internal links.

On a related note it would be good to support something like a set of prerendering param values. E.g. I would like to render all my pages for lang in ['', 'en'] even if no internal link points to some of them. Something like the ['*'] in config.kit.prerender.entries, but with fixed param values. In the case of prefix param it might even be something like ['/*', '/en/*'] in the same config param.

@dummdidumm dummdidumm added the bug Something isn't working label Jan 17, 2023
@Rich-Harris
Copy link
Member

Yeah, I think base: '/' should be allowed. By default we do want to support relative links when there's no base path, since it makes stuff more portable (see e.g. #595 for motivating use cases), but allowing base: '/' would be a sensible way to opt out of that.

We could in theory treat the existence of a <base> element (either in src/app.html or the rendered page content) as a signal to use absolute asset paths, but at SSR time it would have to be a naive html.includes('<base') check which could yield false positives, so specifying the option is preferable I think. Would that suffice in your case?

Accounting for <base> when prerendering is a no-brainer, we should fix that.

@Rich-Harris Rich-Harris added this to the soon milestone Jan 27, 2023
@Rich-Harris Rich-Harris added low hanging fruit ready to implement please submit PRs for these issues! labels Jan 27, 2023
@tontonsb
Copy link
Author

Thanks for looking into this!

Well, on most cases with a present <base> the existing relative paths wouldn't work, so making asset paths absolute makes sense. But if you suspect you won't be able to avoid false positives, maybe just do if (html.includes('<base')) console.log("Warning: you have <base> so you might want to opt for '/' in the base or asset base")?

I suppose the best of both worlds would be to take the base href into account when creating the relative paths, but that's likely complicated with the current crawler.

Btw it might be useful to expose the function that generates the relative paths. Things like <base href="../../en/"> would help with path prefixing but still allow putting the project in a directory.

@Rich-Harris
Copy link
Member

A warning is a good idea, I like that.

The <base href="../../en"> thing could be done in userland like so:

<base href="{'../'.repeat($page.url.pathname.split('/').length - 1)}/{lang}/">

A tiny bit clunky but arguably simple enough that we don't need to add it to the framework

@GitRowin
Copy link
Contributor

I am also in need of base: '/'. I have a 404 page in my root folder that I serve when the requested file could not be found. This breaks when the requested file is not in the root folder, e.g. /non-existent/file, because ./_app/immutable/start-2d81a9c7.js now resolves to /non-existent/_app/immutable/start-2d81a9c7.js, which obviously does not exist.

This setup works fine with Next.js because it uses absolute paths by default: /_next/static/chunks/main-f11614d8aa7ee555.js.

Unrelated to OP's issue, but related to mine: SvelteKit renders +error.svelte when the requested file could not be found. Is there a way to make it render the beforementioned 404 page without doing the following inside +error.svelte?

...
{#if $page.status === 404}
	<svelte:component this={NotFound} />
{:else}
...

It is not a big deal but I thought I should ask.

Rich-Harris added a commit that referenced this issue Mar 1, 2023
dummdidumm pushed a commit that referenced this issue Mar 6, 2023
together with setting config.kit.paths.relative to false this closes #8559
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working low hanging fruit ready to implement please submit PRs for these issues!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants