-
Notifications
You must be signed in to change notification settings - Fork 523
fix(builtin): linker no longer makes node_modules symlink to the root of the workspace output tree #1797
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
fix(builtin): linker no longer makes node_modules symlink to the root of the workspace output tree #1797
Changes from all commits
30f203a
ff47e3b
11d1723
3742a9f
b56a530
eb42481
4d1b1b7
d65ec15
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,3 +35,9 @@ ng_module( | |
| "@npm//rxjs", | ||
| ], | ||
| ) | ||
|
|
||
| filegroup( | ||
| name = "todos_esm", | ||
| srcs = [":todos"], | ||
| output_group = "es6_sources", | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,3 @@ | ||
| load("//:index.bzl", "copy_to_bin") | ||
|
|
||
| package(default_visibility = ["//internal/linker/test:__subpackages__"]) | ||
|
|
||
| copy_to_bin( | ||
| name = "copy_to_bin", | ||
| srcs = ["index.js"], | ||
| ) | ||
| exports_files(["index.js"]) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,8 +7,20 @@ const t = require('transitive_static_linked'); | |
| // they should get resolved from the execroot | ||
| const b = require('dynamic_linked'); | ||
| const d = require('@linker_scoped/dynamic_linked'); | ||
| // We've always supported `require('my_workspace')` for absolute imports like Google does it | ||
| const c = require('build_bazel_rules_nodejs/internal/linker/test/integration/absolute_import'); | ||
|
|
||
| let c; | ||
| try { | ||
| // As of 2.0, we no longer support `require('my_workspace/path/to/output/file.js')` for absolute | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would be nice to link to this spot in the BREAKING CHANGE |
||
| // imports | ||
| c = require('build_bazel_rules_nodejs/internal/linker/test/integration/absolute_import'); | ||
| console.error('should have failed'); | ||
| process.exit(1); | ||
| } catch (_) { | ||
| // You now need to use the runfiles helper library to resolve absolute workspace imports | ||
| const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']); | ||
| c = require(runfiles.resolve( | ||
| 'build_bazel_rules_nodejs/internal/linker/test/integration/absolute_import')); | ||
| } | ||
|
|
||
| // Third-party package installed in the root node_modules | ||
| const semver = require('semver'); | ||
|
|
||
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was this just a warning you're fixing or is it related to the change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 where did that symbol come from before I added that load?