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

Code imported node_modules from execroot instead of bin causing weird issues #590

Closed
calebmer opened this issue Nov 4, 2022 · 2 comments

Comments

@calebmer
Copy link

calebmer commented Nov 4, 2022

Today I ran into a difficult to debug issue. An esbuild rule was bundling two versions of a module. Eventually I discovered that I had missed adding a module to a ts_project()'s deps. So esbuild was bundling a version from both workspace/node_modules (and all its dependencies) and a version from workspace/bazel-bin/node_modules (and all its dependencies).

esbuild was able to find the execroot from its sandbox because the source files were symlinked from the execroot in the sandbox.

This seems like it could be a problem in many contexts. A missing dep means code silently imports from the workspace node_modules instead of reporting an error.


As for solutions, in the Google JavaScript rules (which I'm familiar with from another project) they were able to have Bazel manage node_modules. Is there a way to get this ruleset to do the same? Maybe something like a symlink to a bazel-* folder?

That would still silently accept broken code but at least the node_modules wouldn't be duplicated since they're identical?

@gregmagolan
Copy link
Member

There is a managed_directories feature in Bazel that we put into use in rules_nodejs a while ago so the yarn_install repository rule would install node_modules into the source tree and the external repository would use a symlink to that node_modules. The feature was a bit hacky under the hood and the Bazel team has deprecated it now and it will be removed in the future.

You could manually symlink a node_modules in your source tree into the one in bazel-bin as part of your workflow outside of Bazel.

The esbuild rule currently escapes the sandbox because it is a go program and so doesn't use our node fs patches which keep node from escaping: aspect-build/rules_esbuild#58. The fix will likely require some changes to esbuild itself to provide the right hooks into their resolver. Something we'd like to fix but our time is limited for open source work and we don't currently have any funding for the issue.

You can in theory keep esbuild in the execroot and not let it escape into the user's workspace by making sure all of its inputs are copied into the output tree with copy_to_bin: https://docs.aspect.build/aspect-build/bazel-lib/v1.9.2/docs/copy_to_bin-docgen.html#copy_to_bin

@gregmagolan
Copy link
Member

@calebmer Did that answer your question?

Thinking about this some more, because the node_modules in the bazel-bin folder is lazy, meaning it is not created unless you build a target that requires a node_modules, there isn't really a good place to make a programmatic a symlink in the user's source tree to the bazel-bin output tree node_modules. In rules_nodejs, this could be done in yarn_install but in rules_js npm_translate_lock doesn't actually create any node_modules trees since they are build actions.

Closing this since the only solution is to create this symlink as part of a custom workflow outside of Bazel. Perhaps if this is something useful it could be a feature request or a plugin to our open-source Aspect CLI: https://github.com/aspect-build/aspect-cli. This is a wrapper around Bazel similar to Bazelisk but it adds additional features and commands and a plugin system so it is extensible.

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

No branches or pull requests

2 participants