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

fix(webpack): fixed isolatedConfig: false option not composing plugins #20678

Merged
merged 1 commit into from
Dec 11, 2023

Conversation

TriPSs
Copy link
Contributor

@TriPSs TriPSs commented Dec 9, 2023

Current Behavior

When compiling a node project that has isolatedConfig set to false and does not provide an webpack config this error is thrown:

 >  NX   Invalid configuration object. Webpack has been initialized using a configuration object that does not match the API schema.

    - configuration has an unknown property 'nxWebpackComposablePlugin'. These properties are valid:
      object { amd?, bail?, cache?, context?, dependencies?, devServer?, devtool?, entry?, experiments?, extends?, externals?, externalsPresets?, externalsType?, ignoreWarnings?, infrastructureLogging?, loader?, mode?, module?, name?, node?, optimization?, output?, parallelism?, performance?, plugins?, profile?, recordsInputPath?, recordsOutputPath?, recordsPath?, resolve?, resolveLoader?, snapshot?, stats?, target?, watch?, watchOptions? }
      -> Options object as provided by the user.
      For typos: please correct them.
      For loader options: webpack >= v2.0.0 no longer allows custom properties in configuration.
        Loaders should be updated to allow passing options via loader options in module.rules.
        Until loaders are updated one can use the LoaderOptionsPlugin to pass these options to the loader:
        plugins: [
          new webpack.LoaderOptionsPlugin({
            // test: /\.xxx$/, // may apply this only for some modules
            options: {
              nxWebpackComposablePlugin: …
            }
          })
        ]
   Pass --verbose to see the stacktrace.

Expected Behavior

For the default plugins to be added correctly

Related Issue(s)

@TriPSs TriPSs requested a review from a team as a code owner December 9, 2023 13:22
@TriPSs TriPSs requested a review from JamesHenry December 9, 2023 13:22
Copy link

vercel bot commented Dec 9, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated (UTC)
nx-dev ⬜️ Ignored (Inspect) Visit Preview Dec 11, 2023 8:44pm

@TriPSs
Copy link
Contributor Author

TriPSs commented Dec 9, 2023

@JamesHenry I also noticed that with isolatedConfig set to false it now does withNx and withWeb, which causes issues for Node projects. This was not the case in the old implementation:

const configure =
options.target === 'web'
? composePluginsSync(withNx(), withWeb())
: withNx();

Shall I re-add that method to this file to restore that logic?

@jaysoo
Copy link
Member

jaysoo commented Dec 11, 2023

@JamesHenry I also noticed that with isolatedConfig set to false it now does withNx and withWeb, which causes issues for Node projects. This was not the case in the old implementation:

const configure =
options.target === 'web'
? composePluginsSync(withNx(), withWeb())
: withNx();

Shall I re-add that method to this file to restore that logic?

I'll add it an get it pushed through.

@jaysoo jaysoo force-pushed the master branch 2 times, most recently from cc2e0e3 to f03ec46 Compare December 11, 2023 20:21
@jaysoo jaysoo enabled auto-merge (squash) December 11, 2023 21:03
@jaysoo jaysoo merged commit 38ad952 into nrwl:master Dec 11, 2023
6 checks passed
meeroslav pushed a commit to meeroslav/nx that referenced this pull request Dec 14, 2023
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants