fix(@angular/cli): resolve in all available node_modules #6475
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
clydin
requested changes
May 26, 2017
| @@ -67,10 +68,10 @@ export function getCommonConfig(wco: WebpackConfigOptions) { | |||
| devtool: buildOptions.sourcemaps ? 'source-map' : false, | |||
| resolve: { | |||
| extensions: ['.ts', '.js'], | |||
| modules: ['node_modules', nodeModules], | |||
| modules: [appRoot].concat(nodeModules), | |||
There was a problem hiding this comment.
This value is correct. Please revert.
| }, | ||
| resolveLoader: { | ||
| modules: [nodeModules] | ||
| modules: nodeModules |
There was a problem hiding this comment.
This should be
resolveLoader: {
modules: [nodeModules, 'node_modules']
| }, | ||
| resolveLoader: { | ||
| modules: [nodeModules] | ||
| modules: nodeModules | ||
| }, | ||
| context: projectRoot, |
There was a problem hiding this comment.
Change this to:
context: __dirname,
| @@ -23,7 +23,8 @@ export function getCommonConfig(wco: WebpackConfigOptions) { | |||
| const { projectRoot, buildOptions, appConfig } = wco; | |||
|
|
|||
| const appRoot = path.resolve(projectRoot, appConfig.root); | |||
| const nodeModules = path.resolve(projectRoot, 'node_modules'); | |||
| const projectRootNodeModules = path.resolve(projectRoot, 'node_modules'); | |||
| const nodeModules = module.paths.slice(0, module.paths.indexOf(projectRootNodeModules) + 1); | |||
There was a problem hiding this comment.
This isn't necessary (see comments below) and uses an undocumented API.
douglasduteil
force-pushed
the
fix-angular-cli-resolve-in-all-available-node-modules
branch
from
30fb86c to
8fec989
Compare
|
Thanks for you feedback @clydin |
douglasduteil
force-pushed
the
fix-angular-cli-resolve-in-all-available-node-modules
branch
from
f4b8524 to
b972943
Compare
clydin
reviewed
May 26, 2017
| @@ -67,12 +67,12 @@ export function getCommonConfig(wco: WebpackConfigOptions) { | |||
| devtool: buildOptions.sourcemaps ? 'source-map' : false, | |||
| resolve: { | |||
| extensions: ['.ts', '.js'], | |||
| modules: ['node_modules', nodeModules], | |||
| modules: [nodeModules, 'node_modules'] | |||
There was a problem hiding this comment.
Keep this one unchanged. Standard node module resolution needs to take precedence to avoid incorrect transitive package versions from being resolved.
clydin
requested changes
May 27, 2017
|
|
||
| interface NodeModule { | ||
| paths: string[]; | ||
| } |
There was a problem hiding this comment.
Please revert this file. This isn't needed anymore.
douglasduteil
force-pushed
the
fix-angular-cli-resolve-in-all-available-node-modules
branch
from
b972943 to
8fcd402
Compare
When @angular/cli dependencies (like @ngtool/webpack for example) are installed in its node_modules (as node_modules/@angular/cli/node_modules for example) webpack isn't seeing them.
douglasduteil
force-pushed
the
fix-angular-cli-resolve-in-all-available-node-modules
branch
from
8fcd402 to
93e584b
Compare
douglasduteil
deleted the
fix-angular-cli-resolve-in-all-available-node-modules
branch
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When @angular/cli dependencies (like @ngtool/webpack for example) are installed in its node_modules (as node_modules/@angular/cli/node_modules for example) webpack isn't seeing them.
Actually I experienced it when trying to use lerna with ng-cli. As Lerna try to be smart about how the node_modules are shared between different packages, it's installing the
@angular/clidependencies in the@angular/cli/node_modules