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

refactor: set symlink_node_modules default to False in yarn_install and npm_install #3214

Conversation

gregmagolan
Copy link
Collaborator

Adds section to docs to explain why using symlink_node_modules is not recommended.

@gregmagolan gregmagolan force-pushed the symlink_node_modules_default_to_false branch 3 times, most recently from 5540a78 to 4a070c7 Compare January 11, 2022 08:09
@gregmagolan gregmagolan force-pushed the symlink_node_modules_default_to_false branch 7 times, most recently from 3ade688 to d35ce7b Compare January 11, 2022 09:24
@@ -20,6 +17,7 @@ load("@build_bazel_rules_nodejs//:index.bzl", "yarn_install")
yarn_install(
# Name this npm so that Bazel Label references look like @npm//package
name = "npm",
data = ["//:patches/jest-haste-map+26.6.2.patch"],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, we should have updated jest so this patch isn't needed, I'll TAL

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea. will do in a follow-up across the repo

@gregmagolan gregmagolan force-pushed the symlink_node_modules_default_to_false branch from d35ce7b to 7485180 Compare January 11, 2022 15:48
@gregmagolan gregmagolan merged commit e1535cc into bazel-contrib:5.x Jan 11, 2022
devversion added a commit to devversion/dev-infra that referenced this pull request Mar 2, 2022
As of RNJ v5, the node modules are no longer symlinked by default. Rather
a new separate install is maintained inside Bazel. This was a little
controversial in the past since it made debugging harder, and required
more IO locally/or even downloads depending on how Yarn is cached/configured
locally.

With the current setup, symlinks are no longer enabled and we can remove
the managed directory configuration.

bazel-contrib/rules_nodejs#3214
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants