Disable resolve.symlinks.

[joshwilsonvu]

Hi all,

I have a non-monorepo use case where I'd like to import ES6 files from outside of the src folder via symlink, as recommended by a CRA error message.

Importing through a symlink gives me this issue, where the file is imported but not processed with the rest of the code under src/, as one would expect. My PR seems to solve it by treating symlinked files as if they were under src/, according to the well-received solution here.

Demo repo, with screenshots. The solution works at least for files under the project root.

Fixes #3547.

[facebook-github-bot]

Hi joshwilsonvu! Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[ianschmitz]

Seems legit. Would be good to have another maintainer see if they can think of any downsides given our setup.

[mrmckeb]

This looks good to me too, but I'm not sure how well our CI covers this.

[1st]

Have you checked, does it still work with external code?

[await-ovo]

any updates ？

[ankon]

My use case: I am working on an application that uses create-react-app, and we do not want to eject -- for one because there are strong warnings against it, and for a second reason is that we would never be able to replicate the knowledge that goes into it.

But, we also want to provide some of the components we create as open-source, which means there is times where I need to work with sources in both the app repository and the component(s) director(y|ies).

Right now my workflow to achieve that is painful:

1. Build the component and run npm link, then remove the node_modules directory.
2. Link the component into the app, then do bits and pieces in the app. Try-out smaller changes directly by modifying the generated .js code.
3. Eventually switch back to the component: Run npm install to restore the node_modules directory. At this the app won't work anymore because of conflicts in react versions etc that lead to fun such as "invalid hook calls".
4. Build tests and storybook stories for all the things discovered in step 2, then implement them.
5. Move to step 1 to start a new cycle.

As long as the app anyways depends on all the stuff that the component depends that is going to work. It's going to stop working when the component has own dependencies, which one would have to install explicitly into the app's node_modules at that point.

I tried now to do something else: Create a dedicated "exposed" component directory which only has the generated files as well as package.json and the production dependencies, and npm link that instead.

In shell-script:

#!/bin/sh

exposed_directory=${PWD}/exposed

# Clean out the exposed directory and recreate it
rm -rf "${exposed_directory}" && mkdir -p "${exposed_directory}"

# Link the various pieces we need to make it look like our package
for f in src $(node -p '(require("./package.json").files || []).join("\n")'); do
	ln -s "../${f}" "${exposed_directory}/${f}"
done
# Hardlink files such as package.json to prevent smart tools trying to bork things.
for f in package.json; do
	ln "${f}" "${exposed_directory}/${f}"
done
(cd "${exposed_directory}" && NODE_ENV=production npm link)

npm install
npm run build -- --watch

This works ... IFF I apply the changes from this PR.

TL;DR: Please, pretty please, merge this? :)

[zoontek]

Please consider merging this 🙏

[wisammechano]

What is this waiting for?

[vicary]

What's wrong with all these waiting and the lack of communications?

[mrmckeb]

Hi there, I was discussing this today with @iansu and we were wondering if this was actually the right solution to the problem.

We are also unsure of what side-effects this change might have (we haven't tested).

Instead of this change, I think we could instead improve where we check for folders outside of the src directory so that it allows for symlinked folders.

How does that sound?

[vicary]

@mrmckeb My use case is always about node_modules.nosync. On top of import and require on .[jt]sx? files, asset loaders, such as postcss-loader, translates the resulting css imports into real paths which also triggers the src jail.

[mrmckeb]

Excuse my ignorance @vicary, do you mean you're using .nosync to stop node_modules from syncing to iCloud? That's what I found in Google.

If so, can you add some more detail around what how this change affects that?

[vicary]

Excuse my ignorance @vicary, do you mean you're using .nosync to stop node_modules from syncing to iCloud? That's what I found in Google.

The usage usually involves keeping both node_modules as a symlink, and node_modules.nosync as a directory in project root. Some modules/loaders always try to resolve the require path such that node_modules always become node_modules.nosync.

Say you import bootstrap in App.js,

import 'bootstrap/scss/bootstrap.scss';

It'll results in the following error,

Failed to compile.

./src/index.css (./node_modules.nosync/css-loader/dist/cjs.js??ref--5-oneOf-4-1!./node_modules.nosync/postcss-loader/src??postcss!./src/index.css)
Module not found: You attempted to import ../node_modules.nosync/css-loader/dist/runtime/api.js which falls outside of the project src/ directory. Relative imports outside of src/ are not supported.

If so, can you add some more detail around what how this change affects that?

So it's either asking all related projects to not resolve symlinks into original paths, or CRA somehow loosen the src jail a bit when the same path is actually resolvable in the allowed paths, aka node_modules, src... etc.

[romgrk-comparative]

I've commented here but I'll reiterate here because I think this should not be merged. The symlinks: false is just a workaround for the real issue: that some source directories aren't processed by webpack when they should. The proper solution is to ensure those directories can be added into the include of the appropriate loaders, not to disable symlink resolution.

Disabling symlink resolution also has the side-effect of breaking pnpm because it uses a single package store for a machine and symlinks required packages into node_modules as appropriate (which is great if you want to avoid wasting 400mb of disk in node_modules for every CRA project you work on). Enabling symlinks: false means enabling a module resolution behavior that is not nodejs-compatible, and webpack should aim to be 100% compatible with nodejs with regards to module resolution rules.

[joshwilsonvu]

I agree with some of the above comments—this is not the solution to the problem, so I’ve closed the PR. Reopen if you will but I’m sure we can solve the issue without breaking a package manager.