-
Notifications
You must be signed in to change notification settings - Fork 522
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: @npm//foobar:foobar__files target no longer includes nested node_modules #1390
fix: @npm//foobar:foobar__files target no longer includes nested node_modules #1390
Conversation
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.
Wouldn't it be less breaking to keep foo__files
containing the same things as today and have the new target be like foo__direct_files
or foo__files_no_deps
?
I thought about that but I think the current What is in the nested node_modules will depend on how things are hoisted which will depend on which package manager was used (yarn/npm) and what other deps are in the package.json. |
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.
In that case what prevents us from removing the foo__all_files
semantics altogether? Do we still depend on that ourselves like the tests here suggest?
The |
do we have to expose both |
|
39d2305
to
164ade9
Compare
…s` & `foo__contents` private
164ade9
to
393922e
Compare
The expected behavior would be that @npm//foobar:foobar__files would just be the files in the package and not the nested node_modules that may or may not be there depending on the behavior of the package installer.
Pre-req for angular/angular#33927
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information