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): config migrations should account for different configurations #14672

Merged
merged 1 commit into from
Jan 27, 2023

Conversation

jaysoo
Copy link
Member

@jaysoo jaysoo commented Jan 27, 2023

This PR fixes an issue where different configurations (e.g. deveopment and production) can mess up migrations between @nrwl/webpack and @nrwl/react.

For example, @nrwl/react migrates a custom webpack.config.js correctly, but now @nrwl/webpack looks at the options, which is like this:

build: {
  ...
  "options": {
     "isolatedConfig": true,
     "main": "myapp/main.tsx",
    "webpackConfig": "myapp/webpack.alreadymigrated.config.js"
  },
  "configurations": {
     "production": {
        "webpackConfig": "myapp/webpack.alreadymigrated.prod.config.js"
      }
  }

And for production configuration, the new isolatedConfig flag is not set, and there is a custom webpackConfig so @nrwl/webpack performs a migration that isn't needed.

The fix is make sure we keep track of the default options, and if the default options need to be migrated, then other configurations may also need to be migrated. If the default options do not need a migration, then skip the rest of the configurations.

Notes

  • The check for main.tsx needs to be on default options, because each configuration will likely not provide a different main option.
  • Updating project configuration should update the options field for default options, or <configurationName> for things like development and production.
  • Isolated each test case to set up required projects, rather than having everything shared in beforeEach.
  • Two new test cases were added to check the configuration case (one in webpack, one in React).

@vercel
Copy link

vercel bot commented Jan 27, 2023

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

1 Ignored Deployment
Name Status Preview Comments Updated
nx-dev ⬜️ Ignored (Inspect) Jan 27, 2023 at 8:34PM (UTC)

@jaysoo jaysoo force-pushed the fix/webpack-config-migrations branch from fbc837e to dc3f531 Compare January 27, 2023 19:52
@jaysoo jaysoo enabled auto-merge (squash) January 27, 2023 19:54
@jaysoo jaysoo force-pushed the fix/webpack-config-migrations branch 2 times, most recently from eb325c1 to 934a41d Compare January 27, 2023 20:03
@jaysoo jaysoo force-pushed the fix/webpack-config-migrations branch from 934a41d to e47e520 Compare January 27, 2023 20:34
@jaysoo jaysoo merged commit cda00d9 into nrwl:master Jan 27, 2023
@jaysoo jaysoo deleted the fix/webpack-config-migrations branch January 27, 2023 20:54
@github-actions
Copy link

github-actions bot commented Mar 4, 2023

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 Mar 4, 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.

None yet

2 participants