refactor: set symlink_node_modules default to False in yarn_install a…

…nd npm_install

WORKSPACE

@@ -19,9 +19,6 @@ workspace(
         "@cypress_deps": ["packages/cypress/test/node_modules"],
         "@internal_npm_install_test_patches_npm_symlinked": ["internal/npm_install/test/patches_npm_symlinked/node_modules"],
         "@internal_npm_install_test_patches_yarn_symlinked": ["internal/npm_install/test/patches_yarn_symlinked/node_modules"],
-        "@internal_test_multi_linker_sub_deps": ["internal/linker/test/multi_linker/sub/node_modules"],
-        "@npm": ["node_modules"],
-        "@npm_node_patches": ["packages/node-patches/node_modules"],
     },
 )
 

docs/Built-ins.md

@@ -899,6 +899,14 @@ Defaults to `True`
 
 (*Boolean*): Turn symlinking of node_modules on
 
+When False, the package manager will run in the external repository
+created by this rule.
+This requires that any files required for it to run should be listed in the
+`data` attribute. These files would include things like patch files that are
+read by a postinstall lifecycle hook such as the `patch-package` package uses.
+`package.json` and the lock file are already specified in dedicated attributes
+of this rule and do not need to be included in the `data`.
+
 When True, we run the package manager (npm or yarn) with the working directory
 set in your source tree, in the folder containing the package.json file.
 The resulting `node_modules` folder in the source tree will be symlinked to the
@@ -930,15 +938,7 @@ Using managed_directories will mean that
 2. if the `node_modules` folder is deleted from the source tree, Bazel will re-run the
    repository rule that creates it again on the next run.
 
-When False, the package manager will run in the external repository
-created by this rule.
-This requires that any files required for it to run should be listed in the
-`data` attribute. These files would include things like patch files that are
-read by a postinstall lifecycle hook such as the `patch-package` package uses.
-`package.json` and the lock file are already specified in dedicated attributes
-of this rule and do not need to be included in the `data`.
-
-Defaults to `True`
+Defaults to `False`
 
 <h4 id="npm_install-timeout">timeout</h4>
 
@@ -1586,6 +1586,14 @@ Defaults to `True`
 
 (*Boolean*): Turn symlinking of node_modules on
 
+When False, the package manager will run in the external repository
+created by this rule.
+This requires that any files required for it to run should be listed in the
+`data` attribute. These files would include things like patch files that are
+read by a postinstall lifecycle hook such as the `patch-package` package uses.
+`package.json` and the lock file are already specified in dedicated attributes
+of this rule and do not need to be included in the `data`.
+
 When True, we run the package manager (npm or yarn) with the working directory
 set in your source tree, in the folder containing the package.json file.
 The resulting `node_modules` folder in the source tree will be symlinked to the
@@ -1617,15 +1625,7 @@ Using managed_directories will mean that
 2. if the `node_modules` folder is deleted from the source tree, Bazel will re-run the
    repository rule that creates it again on the next run.
 
-When False, the package manager will run in the external repository
-created by this rule.
-This requires that any files required for it to run should be listed in the
-`data` attribute. These files would include things like patch files that are
-read by a postinstall lifecycle hook such as the `patch-package` package uses.
-`package.json` and the lock file are already specified in dedicated attributes
-of this rule and do not need to be included in the `data`.
-
-Defaults to `True`
+Defaults to `False`
 
 <h4 id="yarn_install-timeout">timeout</h4>
 

docs/dependencies.md

@@ -29,9 +29,6 @@ This approach also allows you to use the generated fine-grained npm package depe
 which can significantly reduce the number of inputs to actions, making Bazel sand-boxing and
 remote-execution faster if there are a large number of files under `node_modules`.
 
-> Note that as of Bazel 0.26, and with the recommended `managed_directories` attribute on the `workspace` rule in `/WORKSPACE`,
-> the Bazel-managed `node_modules` directory is placed in your workspace root in the standard location used by npm or yarn.
-
 ## Using Bazel-managed dependencies
 
 To have Bazel manage its own copy of `node_modules`, which is useful to avoid
@@ -65,8 +62,28 @@ npm_install(
 > If you don't need to pass any arguments to `node_repositories`,
   you can skip calling that function. `yarn_install` and `npm_install` will do it by default.
 
-You should now add the `@npm` workspace to the `managed_directories` option in the `workspace` rule at the top of the file. This tells Bazel that the `node_modules` directory is special and is managed by the package manager.
-Add the `workspace` rule if it isn't already in your `/WORKSPACE` file.
+### symlink_node_modules and managed_directories
+
+Set `symlink_node_modules` to `True` to configure `npm_install` and
+`yarn_install` to install `node_modules` inside the user workspace and have
+Bazel use the `node_modules` folder in the user workspace for the build via a
+symlink into the external repository it creates.
+
+```python
+load("@build_bazel_rules_nodejs//:index.bzl", "npm_install")
+
+npm_install(
+    name = "npm",
+    package_json = "//:package.json",
+    package_lock_json = "//:package-lock.json",
+    symlink_node_modules = True,
+)
+```
+
+You should now add the `@npm` workspace to the `managed_directories` option in
+the `workspace` rule at the top of the file. This tells Bazel that the
+`node_modules` directory is special and is managed by the package manager. Add
+the `workspace` rule if it isn't already in your `/WORKSPACE` file.
 
 ```python
 workspace(
@@ -75,7 +92,18 @@ workspace(
 )
 ```
 
-As of Bazel 0.26 this feature is still experimental, so also add this line to the `.bazelrc` to opt-in:
+As of rules_nodejs 5.0, `symlink_node_modules` defaults to `False` and using
+`managed_directories` is not recommended. We've found that the benefits of using
+`symlink_node_modules`, which allows bazel to use a `node_modules` directory
+that is in the user workspace, do not outweigh the downsides of the repository
+rules not defining all of their input and of having to re-run the repository
+rule if the user's `node_modules` folder is deleted. On persistent CI machines
+that will run clean the state of the clone between jobs the repository rule will
+run for every job with `symlink_node_modules` enabled. With it disabled, the
+repository rule will only re-run if any of its inputs changes between jobs. 
+
+NB: On older versions of Bazel you may have to add the following flag to your
+`.bazelrc` to enable managed directories.
 
 ```
 common --experimental_allow_incremental_repository_updates
@@ -131,13 +159,7 @@ and npm deps, `yarn_install` (or `npm_install`) can be called separately for
 each.
 
 ```python
-workspace(
-    name = "my_wksp",
-    managed_directories = {
-        "@app1_npm": ["app1/node_modules"],
-        "@app2_npm": ["app2/node_modules"],
-    },
-)
+workspace(name = "my_wksp")
 
 yarn_install(
     name = "app1_npm",

e2e/bazel_managed_deps/WORKSPACE

@@ -12,10 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-workspace(
-    name = "e2e_bazel_managed_deps",
-    managed_directories = {"@npm": ["node_modules"]},
-)
+workspace(name = "e2e_bazel_managed_deps")
 
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 

e2e/concatjs_devserver/WORKSPACE

@@ -12,10 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-workspace(
-    name = "e2e_concatjs_devserver",
-    managed_directories = {"@npm": ["node_modules"]},
-)
+workspace(name = "e2e_concatjs_devserver")
 
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 

e2e/coverage/WORKSPACE

@@ -12,10 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-workspace(
-    name = "e2e_coverage",
-    managed_directories = {"@npm": ["node_modules"]},
-)
+workspace(name = "e2e_coverage")
 
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 

e2e/fine_grained_symlinks/WORKSPACE

@@ -1,7 +1,4 @@
-workspace(
-    name = "e2e_fine_grained_symlinks",
-    managed_directories = {"@npm": ["node_modules"]},
-)
+workspace(name = "e2e_fine_grained_symlinks")
 
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 

e2e/jasmine/WORKSPACE

@@ -12,10 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-workspace(
-    name = "e2e_jasmine",
-    managed_directories = {"@npm": ["node_modules"]},
-)
+workspace(name = "e2e_jasmine")
 
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 

e2e/node_loader_no_preserve_symlinks/.bazelrc

@@ -1 +1,3 @@
 import %workspace%/../../common.bazelrc
+
+build --@build_bazel_rules_nodejs//nodejs:default_args=""

e2e/node_loader_no_preserve_symlinks/BUILD.bazel

@@ -9,5 +9,4 @@ nodejs_test(
         "@npm//:node_modules",
     ],
     entry_point = ":node_loader_test.spec.js",
-    templated_args = ["--nobazel_node_patches"],
 )

e2e/node_loader_no_preserve_symlinks/WORKSPACE

@@ -1,9 +1,4 @@
-workspace(
-    name = "e2e_node_loader_no_preserve_symlinks",
-    managed_directories = {
-        "@npm": ["node_modules"],
-    },
-)
+workspace(name = "e2e_node_loader_no_preserve_symlinks")
 
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 

e2e/node_loader_preserve_symlinks/WORKSPACE

@@ -1,9 +1,4 @@
-workspace(
-    name = "e2e_node_loader_preserve_symlinks",
-    managed_directories = {
-        "@npm": ["node_modules"],
-    },
-)
+workspace(name = "e2e_node_loader_preserve_symlinks")
 
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 

e2e/nodejs_image/WORKSPACE

@@ -12,10 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-workspace(
-    name = "e2e_nodejs_image",
-    managed_directories = {"@npm": ["node_modules"]},
-)
+workspace(name = "e2e_nodejs_image")
 
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 

e2e/nodejs_repository/WORKSPACE

@@ -1,7 +1,4 @@
-workspace(
-    name = "nodejs_repository",
-    managed_directories = {"@npm": ["node_modules"]},
-)
+workspace(name = "nodejs_repository")
 
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 

e2e/packages/WORKSPACE

@@ -25,7 +25,6 @@ npm_install(
     data = ["//:postinstall.js"],
     package_json = "//:npm1/package.json",
     package_lock_json = "//:npm1/package-lock.json",
-    symlink_node_modules = False,
 )
 
 npm_install(
@@ -34,15 +33,13 @@ npm_install(
     data = ["//:postinstall.js"],
     package_json = "//:npm2/package.json",
     package_lock_json = "//:npm2/package-lock.json",
-    symlink_node_modules = False,
 )
 
 yarn_install(
     name = "e2e_packages_yarn_install",
     args = ["--prod"],
     data = ["//:postinstall.js"],
     package_json = "//:yarn1/package.json",
-    symlink_node_modules = False,
     yarn_lock = "//:yarn1/yarn.lock",
 )
 
@@ -51,6 +48,5 @@ yarn_install(
     args = ["--prod"],
     data = ["//:postinstall.js"],
     package_json = "//:yarn2/package.json",
-    symlink_node_modules = False,
     yarn_lock = "//:yarn2/yarn.lock",
 )

e2e/symlinked_node_modules_npm/WORKSPACE

@@ -1,7 +1,4 @@
-workspace(
-    name = "e2e_symlinked_node_modules_yarn",
-    managed_directories = {"@npm": ["node_modules"]},
-)
+workspace(name = "e2e_symlinked_node_modules_yarn")
 
 # There are some assertions that can only by made by running tests from within
 # this nested repository folder. These assertions are run in CI so we use

e2e/symlinked_node_modules_yarn/WORKSPACE

@@ -1,7 +1,4 @@
-workspace(
-    name = "e2e_symlinked_node_modules_yarn",
-    managed_directories = {"@npm": ["node_modules"]},
-)
+workspace(name = "e2e_symlinked_node_modules_yarn")
 
 # There are some assertions that can only by made by running tests from within
 # this nested repository folder. These assertions are run in CI so we use

e2e/typescript/WORKSPACE

@@ -12,10 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-workspace(
-    name = "e2e_typescript",
-    managed_directories = {"@npm": ["node_modules"]},
-)
+workspace(name = "e2e_typescript")
 
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 

e2e/webapp/WORKSPACE

@@ -12,10 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-workspace(
-    name = "e2e_webapp",
-    managed_directories = {"@npm": ["node_modules"]},
-)
+workspace(name = "e2e_webapp")
 
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 

examples/angular/WORKSPACE

@@ -5,10 +5,7 @@
 # ESModule imports (and TypeScript imports) can be absolute starting with the workspace name.
 # The name of the workspace should match the npm package where we publish, so that these
 # imports also make sense when referencing the published package.
-workspace(
-    name = "examples_angular",
-    managed_directories = {"@npm": ["node_modules"]},
-)
+workspace(name = "examples_angular")
 
 # These rules are built-into Bazel but we need to load them first to download more rules
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

examples/angular_bazel_architect/WORKSPACE

@@ -29,11 +29,6 @@ yarn_install(
     name = "npm",
     data = ["//:patches/@angular-devkit+architect-cli+0.1102.2.patch"],
     package_json = "//:package.json",
-    # Turn off symlink_node_modules here as it causes extreme flakiness on buildkite
-    # macos CI with missing files in node_modules.
-    # TODO: track down the root cause of the flakiness; it may be something to
-    # do with how the Bazel team has setup their macos virtualization.
-    symlink_node_modules = False,
     yarn_lock = "//:yarn.lock",
 )
 

examples/app/WORKSPACE

@@ -12,10 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-workspace(
-    name = "examples_app",
-    managed_directories = {"@npm": ["node_modules"]},
-)
+workspace(name = "examples_app")
 
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 

examples/closure/WORKSPACE

@@ -12,10 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-workspace(
-    name = "examples_closure",
-    managed_directories = {"@npm": ["node_modules"]},
-)
+workspace(name = "examples_closure")
 
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 

examples/create-react-app/WORKSPACE

@@ -1,7 +1,4 @@
-workspace(
-    name = "create_react_app",
-    managed_directories = {"@npm": ["node_modules"]},
-)
+workspace(name = "create_react_app")
 
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 
@@ -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"],
     exports_directories_only = True,
     package_json = "//:package.json",
     yarn_lock = "//:yarn.lock",