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

Symlinked app pages fail to resolve in DEV but not PRODUCTION #53175

Open
1 task done
cseitz opened this issue Jul 25, 2023 · 9 comments
Open
1 task done

Symlinked app pages fail to resolve in DEV but not PRODUCTION #53175

cseitz opened this issue Jul 25, 2023 · 9 comments
Labels
bug Issue was opened via the bug report template. Navigation Related to Next.js linking (e.g., <Link>) and navigation.

Comments

@cseitz
Copy link

cseitz commented Jul 25, 2023

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
      Platform: linux
      Arch: x64
      Version: #1 SMP Fri Jan 27 02:56:13 UTC 2023
    Binaries:
      Node: 18.12.1
      npm: 9.7.1
      Yarn: 1.22.18
      pnpm: 8.6.6
    Relevant Packages:
      next: 13.4.13-canary.1
      eslint-config-next: 13.4.12
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 5.1.6
    Next.js Config:
      output: N/A

Which area(s) of Next.js are affected? (leave empty if unsure)

App Router, Routing (next/router, next/navigation, next/link), SWC transpilation

Link to the code that reproduces this issue or a replay of the bug

https://github.com/cseitz/next-13.4.13-bug-symlinked-app-dir/

To Reproduce

Running next dev and visiting localhost:3000/test1 will 404 NOT FOUND.

However, running next build and next start and visiting localhost:3000/test1 will work just fine.

(Additional details in the README of my repo)

Describe the Bug

I am using a symlinked directory within my app folder. This symlink route 404's in development, but works perfectly in production.

  • I have a directory, src/features/test1/app, that is symlinked into src/app/(features)/(test1)
  • I symlinked this folder via ln -sr "./src/features/test1/app" "./src/app/(features)/(test1)"
  • Running next dev and visiting localhost:3000/test1 will 404 NOT FOUND

Expected Behavior

When running via next dev:

  • Visiting localhost:3000/test1 should render src/features/test1/app/test1/page.tsx, which is located within the symlinked folder
  • That next dev mirrors the behavior of next build && next start (which is proven to support this symlink feature as it works in production)

Which browser are you using? (if relevant)

Chrome 114.0.5735.199

How are you deploying your application? (if relevant)

No response

@cseitz cseitz added the bug Issue was opened via the bug report template. label Jul 25, 2023
@github-actions github-actions bot added Navigation Related to Next.js linking (e.g., <Link>) and navigation. SWC Related to minification/transpilation in Next.js. labels Jul 25, 2023
@cseitz
Copy link
Author

cseitz commented Aug 21, 2023

Problem Identified

It looks like we're using watchpack in server/lib/router-utils/setup-dev.ts. Watchpack's internal DirectoryWatcher is using fs.lstat instead of fs.stat, which results in symbolic directory links being treated as symlink files rather than the directory the symlink points to.

Viable Solutions

I was able to make a variety of alterations to both watchpack and next.js's code and was able to arrive at a working solution to this problem.

The following potential solutions to this issue are detailed below. Each one has differing levels of complexity and impact on next.js and watchpack itself.

Additionally, all of the following will require the following changes to: server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts:

- 45:     if (file.isDirectory()) {
+ 45:     if (file.isDirectory() || file.isSymbolicLink()) {

A. Fix in Watchpack

We can swap over watchpack's usage of fs.lstat to fs.stat and this solves the issue. However, this would obviously cause issues with existing users of watchpack and we'd likely need to add a new config option to watchpack and we'd have to wait for watchpack to approve the change and publish it for next.js to resolve this issue.

B. Patching Watchpack's DirectoryWatcher

We can create a new class that extends Watchpack's DirectoryWatcher and override its onWatchEvent and doScan functions so that they use fs.stat instead of fs.lstat. However, both of these functions contain significant logic that would need to be copy-pasted and would no longer receive updates if watchpack publishes any to the DirectoryWatcher's onWatchEvent and doScan functions.

C. Watching symbolic directories

The simplest but somewhat crude fix is that we can alter setup-dev.ts so that it scans for symlinked directories and automatically adds them to wp.watch({ directories: [ ... ] }). This would be the easiest to implement.

Resolution

I would be happy to submit a pull request with one of these solutions if someone on the next.js team could provide guidance on how to move forward and which path would be the best route.

Thanks!

@cseitz
Copy link
Author

cseitz commented Sep 29, 2023

For those of you still looking for a solution, I have a temporary workaround.

I was able to use PNPM Patches to make 4 small edits to next.js and watchpack to get things working.

See more details in my Repo (specifically the pnpm-patch branch)
https://github.com/cseitz/next-13.4.13-bug-symlinked-app-dir/tree/pnpm-patch

@silte
Copy link

silte commented Sep 29, 2023

For those of you still looking for a solution, I have a temporary workaround.

I was able to use PNPM Patches to make 4 small edits to next.js and watchpack to get things working.

See more details in my Repo (specifically the pnpm-patch branch) https://github.com/cseitz/next-13.4.13-bug-symlinked-app-dir/tree/pnpm-patch

This patch is compatible also with version 13.5 of Next.js. However, for version 13.5, you only need to apply the patch to watchpack, as Next.js has updated its directory parsing logic to support symlinks.

@bartvandenende-wm
Copy link

bartvandenende-wm commented Dec 19, 2023

created an issue for tracking webpack/watchpack#231, watchpack resolve the symlinks to the final target looks to be the main root cause. Specifically if the symlink target is outside of the src directory.

If a change is accepted in watchpack it would likely require a small patch in NextJs to opt-in to the new symlink resolving here:

An alternative direction would be for NextJs to expose a configuration option that would allow user to specify alternative watch directories, this would work as long as the symlink target is still within the project directory.

const directories = [...pages, ...app]

@cseitz
Copy link
Author

cseitz commented Feb 10, 2024

I just did some testing on Next.js 14.1.0 and this issue is still present and even more difficult to address now.

Next.js 14.0.0 migrated the watchpack@2.4.0 peer dependency into a compiled dependency, meaning one can no longer use a package manager to override this dependency without involving the use of patches.

The solution to this problem is still just changing lstat to stat in 2 places of DirectoryWatcher.js.

Although I appreciate @bartvandenende-wm's attempt to resolve this within watchpack, watchpack has not been updated in 2 years and its README explicitly states:

Concept

...

  • Symlinks are not followed, instead the symlink is watched.

I think its safe to assume that addressing this issue for Next.js within Watchpack itself goes against the objectives of Watchpack and thus will never get merged in. It must be addressed within Next.js directly.

Given that it is possible via running in production, I think we should migrate Next.js's use of Watchpack hot reload using a different implementation; potentially even just a fork of Watchpack with the required modifications.

If a maintainer could help weigh in on this, I would greatly appreciate it. Is there any reason why symlinked app directories shouldn't be a feature of Next.js? If next build is working as intended with functioning symlinks, then I think we need to address this issue within the dev server.

@nahtnam
Copy link

nahtnam commented Mar 14, 2024

I tried with --turbo and that doesn't work either

@kdy1 kdy1 removed the SWC Related to minification/transpilation in Next.js. label Mar 19, 2024
@gaithoben
Copy link

I just did some testing on Next.js 14.1.0 and this issue is still present and even more difficult to address now.

Next.js 14.0.0 migrated the watchpack@2.4.0 peer dependency into a compiled dependency, meaning one can no longer use a package manager to override this dependency without involving the use of patches.

The solution to this problem is still just changing lstat to stat in 2 places of DirectoryWatcher.js.

Although I appreciate @bartvandenende-wm's attempt to resolve this within watchpack, watchpack has not been updated in 2 years and its README explicitly states:

Concept

...

  • Symlinks are not followed, instead the symlink is watched.

I think its safe to assume that addressing this issue for Next.js within Watchpack itself goes against the objectives of Watchpack and thus will never get merged in. It must be addressed within Next.js directly.

Given that it is possible via running in production, I think we should migrate Next.js's use of Watchpack hot reload using a different implementation; potentially even just a fork of Watchpack with the required modifications.

If a maintainer could help weigh in on this, I would greatly appreciate it. Is there any reason why symlinked app directories shouldn't be a feature of Next.js? If next build is working as intended with functioning symlinks, then I think we need to address this issue within the dev server.

Hello @cseitz , Did you manage to get a work around on this issue?

@lyonb96
Copy link

lyonb96 commented Apr 21, 2024

Would love to see a resolution to this issue - I'm looking to use symlinked pages to allow external "modules" to define their own routing inside the site without resorting to copying files. Symlinked components from external directories work perfectly fine with the proper configuration, but symlinked routes still don't.

@nahtnam
Copy link

nahtnam commented Apr 21, 2024

For anyone looking for a workaround, I use cpx2 to just copy directories into the app folder for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue was opened via the bug report template. Navigation Related to Next.js linking (e.g., <Link>) and navigation.
Projects
None yet
Development

No branches or pull requests

7 participants