Routes with umlauts are not found in node builds

[noahsalvi]

Describe the bug

When you create a route that has an umlaut in it, for example "über-uns" and you reload the page, you get a 404 Not found. This only happens when you run the build created by the adapter-node, preview and dev work as expected.

Reproduction

https://github.com/noahsalvi/kit-umlaut-repro

install deps; npm run build; node build/

Logs

http://localhost:3000/hell%C3%B6 404 (Not Found)

System Info

System:
    OS: macOS 11.5
    CPU: (8) x64 Intel(R) Core(TM) i7-8559U CPU @ 2.70GHz
    Memory: 58.57 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.4.0 - /usr/local/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 7.18.1 - /usr/local/bin/npm
  Browsers:
    Chrome: 92.0.4515.131
    Edge: 91.0.864.71
    Firefox: 82.0.3
    Safari: 14.1.2
  npmPackages:
    @sveltejs/adapter-node: next => 1.0.0-next.39 
    @sveltejs/kit: next => 1.0.0-next.146 
    svelte: ^3.34.0 => 3.42.1

Severity

serious, but I can work around it

Additional Information

The problem is that the adapter-node parses the url which encodes the umlaut characters, resulting in the route not being found.

Meaning const parsed = new URL(......) should be removed and replaced with just using req.path and new URLSearchParams(req.query)

I could make a pull request if needed.

[benmccann]

Sure, a PR would be welcomed. Thanks for tracking this down!

[noahsalvi]

Can this be reopened since this issue isn't fixed in "@sveltejs/adapter-node" 1.0.0-next.40 or do I need to make a new issue?

System:
    OS: macOS 11.5
    CPU: (8) x64 Intel(R) Core(TM) i7-8559U CPU @ 2.70GHz
    Memory: 747.07 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.4.0 - /usr/local/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 7.18.1 - /usr/local/bin/npm
  Browsers:
    Chrome: 92.0.4515.131
    Edge: 91.0.864.71
    Firefox: 82.0.3
    Safari: 14.1.2
  npmPackages:
    @sveltejs/adapter-node: 1.0.0-next.40 => 1.0.0-next.40 
    @sveltejs/kit: next => 1.0.0-next.146 
    svelte: ^3.34.0 => 3.42.1 
    ```

[benmccann]

I think there's something weird going on here with npm. It kept giving me old versions of the packages. I had to manually specify the versions:

  "devDependencies": {
    "@sveltejs/adapter-node": "1.0.0-next.40",
    "@sveltejs/kit": "1.0.0-next.147",

After doing that, I'm getting a new error, which looks like an issue with sirv. It looks like sirv can't find the JS file on disk

I get a 404 if I visit http://localhost:3000/_app/pages/hellö.svelte-a480177e.js

Eventhough its in the build directory: build/assets/_app/pages/hellö.svelte-a480177e.js

[benmccann]

The Sirv bug is here: lukeed/sirv#82 (comment)

Vite worked around it here vitejs/vite#1426

[benmccann]

@noahsalvi can you test #2191 and confirm it fixes the issue for you?

[noahsalvi]

@noahsalvi can you test #2191 and confirm it fixes the issue for you?

It works 🎉

[benmccann]

@noahsalvi thanks for helping with this! I just released the fix. It changed a bit since you last tested, so you may want to test once again just to confirm

[noahsalvi]

@noahsalvi thanks for helping with this! I just released the fix. It changed a bit since you last tested, so you may want to test once again just to confirm

I just tested it with my repro with kit 148 and adapter-node 41 and sadly i'm getting the same error as before:
Failed to fetch dynamically imported module: http://localhost:3000/_app/pages/hell%C3%B6.svelte-2ea1f198.js

btw Glad that i was/am helpful. Also a big "thanks" to you for doing so much for this awesome project

[benmccann]

okay 😢 🔫 glad I told you to test again at least

[benmccann]

@lukeed it seems that sirv still isn't working with unicode characters in SvelteKit, but I'm not sure what's going on this time. Any ideas? (my previous workaround no longer helps - not that I'd expect it to now)

You can use https://github.com/noahsalvi/kit-umlaut-repro to test and set the latest versions in package.json:

    "@sveltejs/adapter-node": "1.0.0-next.41",
    "@sveltejs/kit": "1.0.0-next.148",

[lukeed]

Cache maybe? I haven't downloaded or cloned any of this before – so this is all my first time:

git clone git@github.com:noahsalvi/kit-umlaut-repro.git
cd kit-umlaut-repro
# make ben's dependency changes
rm yarn.lock
yarn
yarn build
yarn preview

[benmccann]

You're not quite testing the same thing. After doing yarn build you should run node build for the adapter to be invoked. That being said preview is also failing for me, so it's weird that's working for you. I have "Disable cache" checked on the Network tab in Chrome, so I have no idea why it'd work for you and not us

[lukeed]

I'll try that too, but I meant local/node_modules cache. Make sure you delete the yarn.lock if you havent already

[lukeed]

Ok so node build fails. Unrelated, but wtf why would it be different? eg, for the adapter to be invoked. That defeats the purpose of a "preview" command.

[noahsalvi]

You're not quite testing the same thing. After doing yarn build you should run node build for the adapter to be invoked. That being said preview is also failing for me, so it's weird that's working for you. I have "Disable cache" checked on the Network tab in Chrome, so I have no idea why it'd work for you and not us

preview works for me. Make sure to also delete the build folder before running the build command.

[benmccann]

It does a production build of your JS, etc. But it can't actually run in production on all the various platforms. I don't know how you'd mirror the Vercel, Netlify, platforms, etc. Maybe integrating with their dev tools. But thus far that's been a TODO

[lukeed]

Ok so what I'm hearing is that preview is not very useful for debugging purposes.

And node build is failing consistently & I know why: the combo-check is working correctly, but because the request has passed thru @polka/url, there's a cached object in there. Right now, that un-decoded cache object is being returned from sirv's parse(req, true).pathname step. Easy fix, will publish after my computer forced-restart is done 😆

[lukeed]

Verified locally that the reproduction now works with node build (and preview still, lol)

[benmccann]

Should be fixed with the latest version. Thanks again for the help, Luke

[noahsalvi]

@benmccann
I just noticed that umlauts don't work when nested in a dynamic route e.g. [variable]/ö
Repro: https://github.com/noahsalvi/kit-path-broken-repro

[benmccann]

I made a new issue for that one since there was already a lot of discussion here that's not necessarily related: #2201