Skip to content

FR: When using workspaces, create symlinks in workspace node_modules to root node_modules #5469

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

Closed
dannycochran opened this issue Mar 7, 2018 · 6 comments
Assignees
Labels

Comments

@dannycochran
Copy link

dannycochran commented Mar 7, 2018

Do you want to request a feature or report a bug?

Feature (may be a dupe of #5332, I cannot tell).

What is the current behavior?

Currently when using workspaces without "nohoist", all common node_modules of workspaces end up in the project root.

This is fine for node which will traverse parent directories until it finds the right module, but react-native's build system (and many third party libraries for react-native) often expect node_modules to exist in the workspace node_modules directory with hard-coded paths.

One could use the new "nohoist" feature in the react native workspace, but this starts to defeat the purpose of de-duping dependencies. For instance, I have an adjacent workspace that uses things like react and react-dom, so I'll need to still have two copies of those libraries even though they could be shared.It also would mean any adjacent workspaces that are react-native projects would not be able to de-dupe their dependencies via hoisting.

I've also found that workspaces which use webpack have a hard time with hoisted modules. My existing webpack config doesn't work when modules get hoisted.

I've managed to get everything working but it requires a lot of configuration and manipulating the "nohoist" option to such an extent that I'm not actually saving any time when running yarn install. So, I scrapped it altogether because the complexity was not worth it.

What is the desired behavior?

If sym links existed for each node_module in each workspace (and each node_module dependency), everything could just work. E.g:

  • projectRoot/
    • node_modules/
      - react-native
      - react
    • workspaces/
      • reactNativeApp/
        • node_modules/
          • react-native->(symlink to ../../node_modules/react-native)
          • react->(symlink to ../../node_modules/react)
      • reactWebApp/
        • node_modules/
          • react->(symlink to ../../node_modules/react)

Please mention your node.js, yarn and operating system version.

Node: 8.9.4
Yarn: 1.5.1

Related

#5332

@ghost ghost assigned arcanis Mar 7, 2018
@ghost ghost added the triaged label Mar 7, 2018
@dannycochran
Copy link
Author

Pinging @connectdotz who wrote the nohoist PR, in case they have ideas for a work around to the issue I'm experiencing.

@connectdotz
Copy link
Contributor

Hi,
you asked a good question, we didn't go down to the link implementation mainly because that not all 3rd party libraries handle links properly, even metro (until recently)... choosing between "nohoist not working for some" vs "not being as optimized as hoist", I chose later, considering nohoist meant to be a "workaround" until the 3rd party libraries can be fixed.

Having said that, I recently realized that you can use --link-duplicates flag to hardlink duplicated modules, so even without symlinks you can still save significant disk space. In my test repo, I found it reduced the node_modules size by 50%. unfortunately, there is a bug right now until PR #5442 can be merged.

Meanwhile, you can also try lerna. I believe it uses symlink in its bootstrap --hoist implementation, maybe similar to what you asked.

@dannycochran
Copy link
Author

@connectdotz Thanks for the reply!

--link-duplicates will certainly help, but still requires us to maintain the version across several package.json repos. Lerna also has the same issue:

"Unfortunately, some tooling does not follow the module resolution spec closely, and instead assumes or requires that dependencies are present specificially in the local node_modules directory. To work around this, it is possible to symlink packages from their hoisted top-level location, to individual package node_modules directory. Lerna does not yet do this automatically, and it is recommended instead to work with tool maintainers to migrate to more compatible patterns."

It seems unlikely that the other tools, such as gradle and cocoapods, are going to adjust themselves to conform to how node operates. An optional --use-symlinks flag when running yarn install would be really nice. Are there a lot of technical barriers to a feature like this?

@connectdotz
Copy link
Contributor

--link-duplicates will certainly help, but still requires us to maintain the version across several package.json repos

not sure I get this point... what I was suggesting is to add nohoist in your root package.json, such as nohoist:["your-package/**"], then install with yarn install --link-duplicates. Yarn will leave all your-package's dependencies in your-package/node_modules but without creating duplicate files (inodes) since they will be hardlink. I think hardlink is better than symlink in this context because 1) it provides the same or slightly better space optimization 2) with better 3rd-party tools/libs compatibility as it doesn't require special link traversal logic that symlinks do.

An optional --use-symlinks flag when running yarn install would be really nice. Are there a lot of technical barriers to a feature like this?

technical barriers - not much, it could just use the nohoist scheme to decide the locations, but during the actual write operation, instead of copying the actual files, create symlinks instead. However, as I mentioned earlier, why create symlinks and a new flag when you can already create hardlinks with --link-duplicates and deliver better effect...

Maybe best to experiment the above with a test repo? Feel free to ask if you encountered any problem running it.

@dannycochran
Copy link
Author

@connectdotz -- I think you're right that --link-duplicates is what I'm looking for. Thanks for your help.

I see #5442 was just merged - is it possible to squeeze that into a minor release for 1.5.2?

@connectdotz
Copy link
Contributor

connectdotz commented Mar 8, 2018

the official release will be up for the yarn core team to decide, but you can try the nightly build today

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants