fix(core): resolve webpack loaders with require.resolve()
#3341
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
With strict package managers such as pnpm or Yarn PnP, transitive dependencies are *not* hoisted to the root node_modules folder. This means that a webpack config defined within a package like '@nrwl/cypress' cannot resolve loaders like 'ts-loader', unless 'ts-loader' is declared in the workspace's own package.json. This is a problem because the workspace might define a different version of 'ts-loader', incompatible with the version declared by '@nrwl/cypress/package.json'. The workspace should not need to declare a dependency on 'ts-loader' anyway. See also: * pnpm/pnpm#801 * webpack/webpack#5087
|
@elliottsj Going to have to revert this one since it is causing an issue with apps not being styled. I think you need to install https://github.com/arcanis/pnp-webpack-plugin You can test your changes locally by publishing to your local registry. https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#publishing-to-a-local-registry |
|
@elliottsj Basically styles were not applied, I'm guessing due to the loaders not being found but didn't debug too much. I'll check out your new PR. Thanks! |
Doginal
pushed a commit
to Doginal/nx
that referenced
this pull request
Nov 25, 2020
With strict package managers such as pnpm or Yarn PnP, transitive dependencies are *not* hoisted to the root node_modules folder. This means that a webpack config defined within a package like '@nrwl/cypress' cannot resolve loaders like 'ts-loader', unless 'ts-loader' is declared in the workspace's own package.json. This is a problem because the workspace might define a different version of 'ts-loader', incompatible with the version declared by '@nrwl/cypress/package.json'. The workspace should not need to declare a dependency on 'ts-loader' anyway. See also: * pnpm/pnpm#801 * webpack/webpack#5087
|
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
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.
With strict package managers such as pnpm or Yarn PnP, transitive dependencies are not hoisted to the root node_modules folder. This means that a webpack config defined within a package like '@nrwl/cypress' cannot resolve loaders like 'ts-loader', unless 'ts-loader' is declared in the workspace's own package.json.
This is a problem because the workspace might define a different version of 'ts-loader', incompatible with the version declared by '@nrwl/cypress/package.json'. The workspace should not need to declare a dependency on 'ts-loader' anyway.
By using
require.resolve(), Node.js's module resolution is used instead of webpack's, so this issue is avoided.See also:
Current Behavior
Workspaces using pnpm must declare dependencies on loaders used by
@nrwl/*packages, and manually keep the versions in sync. Or, apublic-hoist-patternmust be defined to hoist webpack loaders.Expected Behavior
pnpm users should not have to declare transitive dependencies or customize package hoisting.