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

πŸ› Symlink routes no longer work (1.0.0-next.432 β€” PR #6174) #6303

Closed
oneezy opened this issue Aug 26, 2022 · 3 comments Β· Fixed by #6796
Closed

πŸ› Symlink routes no longer work (1.0.0-next.432 β€” PR #6174) #6303

oneezy opened this issue Aug 26, 2022 · 3 comments Β· Fixed by #6796
Labels
bug Something isn't working
Milestone

Comments

@oneezy
Copy link

oneezy commented Aug 26, 2022

Describe the bug

Symlink routes / Symbolic link routes are no longer working since Sveltekit version 1.0.0-next.432 (PR #6174)

Reproduction

1.0.0-next.431 (working)
βœ… https://github.com/oneezy/sveltekit-symlink-repro-next-431

1.0.0-next.432 (broke)
❌ https://github.com/oneezy/sveltekit-symlink-repro-next-432

image

Logs

Error: Not found: /symlink
    at resolve (file:///C:/Users/oneezy/Desktop/www/10.Repros/sveltekit-symlink-repro-next-442/node_modules/.pnpm/@sveltejs+kit@1.0.0-next.442_svelte@3.49.0+vite@3.0.9/node_modules/@sveltejs/kit/src/runtime/server/index.js:430:14)
    at Object.handle (file:///C:/Users/oneezy/Desktop/www/10.Repros/sveltekit-symlink-repro-next-442/node_modules/.pnpm/@sveltejs+kit@1.0.0-next.442_svelte@3.49.0+vite@3.0.9/node_modules/@sveltejs/kit/src/vite/dev/index.js:318:66)
    at respond (file:///C:/Users/oneezy/Desktop/www/10.Repros/sveltekit-symlink-repro-next-442/node_modules/.pnpm/@sveltejs+kit@1.0.0-next.442_svelte@3.49.0+vite@3.0.9/node_modules/@sveltejs/kit/src/runtime/server/index.js:213:40)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async file:///C:/Users/oneezy/Desktop/www/10.Repros/sveltekit-symlink-repro-next-442/node_modules/.pnpm/@sveltejs+kit@1.0.0-next.442_svelte@3.49.0+vite@3.0.9/node_modules/@sveltejs/kit/src/vite/dev/index.js:375:22

System Info

System:
    OS: Windows 11
    CPU: (16) x64 Intel(R) Core(TM) i7-10875H CPU @ 2.30GHz
    Memory: 15.50 GB / 31.75 GB
  Binaries:
    Node: 16.13.2 - C:\Program Files\nodejs\node.EXE
    npm: 8.1.2 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: 104.0.5112.102
    Edge: Spartan (44.22000.120.0), Chromium (104.0.1293.63)
    Internet Explorer: 11.0.22000.120
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.66 
    @sveltejs/kit: 1.0.0-next.406 => 1.0.0-next.406 
    svelte: ^3.44.0 => 3.49.0 
    vite: ^3.0.4 => 3.0.9

Severity

blocking all usage of SvelteKit

Additional Information

We use a monorepo with multiple separate apps that we link together via symlink routes so this one's a showstopper for us.

@oneezy oneezy changed the title πŸ› Symlink routes no longer work πŸ› Symlink routes no longer work (1.0.0-next.432) Aug 26, 2022
@oneezy
Copy link
Author

oneezy commented Aug 27, 2022

I believe the issue is somewhere around here:
kit/src/core/sync/create_manifest_data/index.js

Doing a little research I found the symlink issue was fixed by @mikenikles in PR #4957 and recently broke with PR #6174

The Fix (commit eb2ab91)

As per @Conduitry's comment, this would error on symlinks to files, as readdirSync throws when not called on a directory.

There seems to be a problem with Node's fs.readdirSync(dir, { withFileTypes: true }) returning false for isDirectory() on symlinks (open issue at nodejs/node#30646). It doesn't seem likely to get fixed upstream in a timely manner, so reverting to fs.statSync instead of withFileTypes seems like a better solution.

Originally posted by @mrkishi in #4957 (review)


With the recent rewrite from @Rich-Harris I believe this was accidentally written back to the old withFileTypes: true way in create_manifest_data/index.js (lines 159-161)

// kit/src/core/sync/create_manifest_data/index.js (lines 159-161)

const files = fs.readdirSync(dir, {
    withFileTypes: true
});

@oneezy oneezy changed the title πŸ› Symlink routes no longer work (1.0.0-next.432) πŸ› Symlink routes no longer work (since 1.0.0-next.432 β€” PR #6174) Aug 30, 2022
@oneezy oneezy changed the title πŸ› Symlink routes no longer work (since 1.0.0-next.432 β€” PR #6174) πŸ› Symlink routes no longer work (1.0.0-next.432 β€” PR #6174) Aug 30, 2022
@oneezy
Copy link
Author

oneezy commented Aug 31, 2022

#HelpWanted

I hacked away on this a bit but didn't have any luck.
PR #6174 refactored quite a bit of the code from it's previous state.

image

PR #4957 before and after fix for symlinks

image

@myhrmans
Copy link

myhrmans commented Sep 1, 2022

Same issue applies on Linux.

@Rich-Harris Rich-Harris added this to the 1.0 milestone Sep 6, 2022
@Rich-Harris Rich-Harris added the bug Something isn't working label Sep 6, 2022
dummdidumm added a commit that referenced this issue Sep 14, 2022
@dummdidumm dummdidumm mentioned this issue Sep 14, 2022
5 tasks
Rich-Harris added a commit that referenced this issue Sep 20, 2022
* [fix] symlink routes

Fixes #6303

* add test

* support symlinks

* lint

* allow symlinked endpoints

* try this

* ugh

Co-authored-by: Rich Harris <hello@rich-harris.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants