Skip to content

Commit

Permalink
feat(misc)!: handle v20 deprecations in plugins (#28222)
Browse files Browse the repository at this point in the history
This PR removes these from v20 since they were deprecated and slated for
removal:

- `executeWebpackDevServerBuilder` export from `@nx/angular/executors`,
users should use `executeDevServerBuilder`
- `withStylus` util from `@nx/next/plugins/with-stylus` since it was
deprecated in v17 and has just throw an error that users need to use
SASS with Next.js

The `getRollupOptions` function from `@nx/react/plugins/bundle-rollup`
has been deprecated as mention previously and slated for removal in v22.
New users are using inferred targets from Rollup, and existing projects
using this module should run `nx g @nx/rollup:convert-to-inferred` or
manually update rollup config to use `withNx` function.

Also, bumped some deprecation for later in v21:

- Remove inline builds from tsc/swc 
- Changes to SVGR to align with Webpack v5 (e.g. `import ReactComponent
from './img.svg?svgr'`)
- Remove `isolatedConfig` from Webpack executor -- requires a migration
that extracts to a standard webpack config just in case (different from
the original one that extracts to `withNx`)

The ESLint TODOs were rescoped to `TODO(eslint)` and we'll look at it in
further flat config work rather than tying it to an Nx release.

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

## Current Behavior
<!-- This is the behavior we have today -->

## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #
  • Loading branch information
jaysoo authored Oct 2, 2024
1 parent 69109e4 commit 81892b5
Show file tree
Hide file tree
Showing 17 changed files with 22 additions and 49 deletions.
5 changes: 0 additions & 5 deletions docs/shared/mental-model/large-tasks.json
Original file line number Diff line number Diff line change
Expand Up @@ -18495,11 +18495,6 @@
"hash": "d750a6f13f1172e9c033370ead717b3f16d9f93b",
"deps": ["npm:next", "devkit", "npm:@nrwl/next"]
},
{
"file": "packages/next/plugins/with-stylus.ts",
"hash": "f3a6bbd478d02c0e9940e4ebe3ab662456b53ebe",
"deps": ["npm:webpack-merge", "npm:next"]
},
{
"file": "packages/next/project.json",
"hash": "171f3c8158b2682525ffb65dac39edf04578460c"
Expand Down
11 changes: 1 addition & 10 deletions packages/angular/executors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,4 @@ export * from './src/executors/application/application.impl';
export * from './src/executors/extract-i18n/extract-i18n.impl';
export * from './src/executors/module-federation-ssr-dev-server/module-federation-ssr-dev-server.impl';

import { executeDevServerBuilder } from './src/builders/dev-server/dev-server.impl';

export {
// TODO(v20): remove this alias
/**
* @deprecated Use executeDevServerBuilder instead. It will be removed in Nx v20.
*/
executeDevServerBuilder as executeWebpackDevServerBuilder,
executeDevServerBuilder,
};
export { executeDevServerBuilder } from './src/builders/dev-server/dev-server.impl';
4 changes: 2 additions & 2 deletions packages/eslint-plugin/src/configs/javascript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export default {
* previously defined v5 of `@typescript-eslint`. v6 of `@typescript-eslint`
* changed how configurations are defined.
*
* TODO(v20): re-evalute these deviations from @typescript-eslint/recommended in v20 of Nx
* TODO(eslint): re-evalute these deviations from @typescript-eslint/recommended in v20 of Nx
*/
'@typescript-eslint/no-non-null-assertion': 'warn',
'@typescript-eslint/adjacent-overload-signatures': 'error',
Expand All @@ -70,7 +70,7 @@ export default {
* During the migration to use ESLint v9 and typescript-eslint v8 for new workspaces,
* this rule would have created a lot of noise, so we are disabling it by default for now.
*
* TODO(v20): we should make this part of what we re-evaluate in v20
* TODO(eslint): we should make this part of what we re-evaluate in v20
*/
'@typescript-eslint/no-require-imports': 'off',
},
Expand Down
4 changes: 2 additions & 2 deletions packages/eslint-plugin/src/configs/typescript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export default {
* previously defined v5 of `@typescript-eslint`. v6 of `@typescript-eslint`
* changed how configurations are defined.
*
* TODO(v20): re-evalute these deviations from @typescript-eslint/recommended in v20 of Nx
* TODO(eslint): re-evalute these deviations from @typescript-eslint/recommended in v20 of Nx
*/
'@typescript-eslint/no-non-null-assertion': 'warn',
'@typescript-eslint/adjacent-overload-signatures': 'error',
Expand All @@ -53,7 +53,7 @@ export default {
* During the migration to use ESLint v9 and typescript-eslint v8 for new workspaces,
* this rule would have created a lot of noise, so we are disabling it by default for now.
*
* TODO(v20): we should make this part of what we re-evaluate in v20
* TODO(eslint): we should make this part of what we re-evaluate in v20
*/
'@typescript-eslint/no-require-imports': 'off',
},
Expand Down
4 changes: 2 additions & 2 deletions packages/eslint-plugin/src/flat-configs/javascript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export default tseslint.config(
* previously defined v5 of `@typescript-eslint`. v6 of `@typescript-eslint`
* changed how configurations are defined.
*
* TODO(v20): re-evalute these deviations from @typescript-eslint/recommended in v20 of Nx
* TODO(eslint): re-evalute these deviations from @typescript-eslint/recommended in v20 of Nx
*/
'@typescript-eslint/no-non-null-assertion': 'warn',
'@typescript-eslint/adjacent-overload-signatures': 'error',
Expand All @@ -74,7 +74,7 @@ export default tseslint.config(
* During the migration to use ESLint v9 and typescript-eslint v8 for new workspaces,
* this rule would have created a lot of noise, so we are disabling it by default for now.
*
* TODO(v20): we should make this part of what we re-evaluate in v20
* TODO(eslint): we should make this part of what we re-evaluate in v20
*/
'@typescript-eslint/no-require-imports': 'off',
},
Expand Down
4 changes: 2 additions & 2 deletions packages/eslint-plugin/src/flat-configs/typescript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export default tseslint.config(
* previously defined v5 of `@typescript-eslint`. v6 of `@typescript-eslint`
* changed how configurations are defined.
*
* TODO(v20): re-evalute these deviations from @typescript-eslint/recommended in v20 of Nx
* TODO(eslint): re-evalute these deviations from @typescript-eslint/recommended in v20 of Nx
*/
'@typescript-eslint/no-non-null-assertion': 'warn',
'@typescript-eslint/adjacent-overload-signatures': 'error',
Expand All @@ -58,7 +58,7 @@ export default tseslint.config(
* During the migration to use ESLint v9 and typescript-eslint v8 for new workspaces,
* this rule would have created a lot of noise, so we are disabling it by default for now.
*
* TODO(v20): we should make this part of what we re-evaluate in v20
* TODO(eslint): we should make this part of what we re-evaluate in v20
*/
'@typescript-eslint/no-require-imports': 'off',
},
Expand Down
2 changes: 1 addition & 1 deletion packages/js/babel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ module.exports = function (api: any, options: NxWebBabelPresetOptions = {}) {

// Determine settings for `@babel//babel-plugin-transform-class-properties`,
// so that we can sync the `loose` option with `@babel/preset-env`.
// TODO(v20): Remove classProperties since it's no longer needed, now that the class props transform is in preset-env.
// TODO(v21): Remove classProperties since it's no longer needed, now that the class props transform is in preset-env.
const loose = options.classProperties?.loose ?? options.loose ?? true;
if (options.classProperties) {
logger.warn(
Expand Down
2 changes: 1 addition & 1 deletion packages/js/src/utils/schema.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,6 @@ export interface NormalizedSwcExecutorOptions
swcCliOptions: SwcCliOptions;
tmpSwcrcPath: string;
sourceRoot?: string;
// TODO(v20): remove inline feature
// TODO(v21): remove inline feature
inline?: boolean;
}
2 changes: 1 addition & 1 deletion packages/js/src/utils/swc/compile-swc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ function getSwcCmd(
) {
const swcCLI = require.resolve('@swc/cli/bin/swc.js');
let inputDir: string;
// TODO(v20): remove inline feature
// TODO(v21): remove inline feature
if (inline) {
inputDir = originalProjectRoot.split('/')[0];
} else {
Expand Down
2 changes: 1 addition & 1 deletion packages/next/plugins/with-nx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ export function getNextConfig(

const svgrOptions =
typeof nx?.svgr === 'object' ? nx.svgr : defaultSvgrOptions;
// TODO(v20): Remove file-loader and use `?react` querystring to differentiate between asset and SVGR.
// TODO(v21): Remove file-loader and use `?react` querystring to differentiate between asset and SVGR.
// It should be:
// use: [{
// test: /\.svg$/i,
Expand Down
16 changes: 0 additions & 16 deletions packages/next/plugins/with-stylus.ts

This file was deleted.

2 changes: 1 addition & 1 deletion packages/react/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { NxReactWebpackPlugin as _NxReactWebpackPlugin } from './plugins/nx-react-webpack-plugin/nx-react-webpack-plugin';

// TODO(v20): Remove this in favor of deep imports in order to load configs faster (150-200ms faster).
// TODO(v21): Remove this in favor of deep imports in order to load configs faster (150-200ms faster).
/** @deprecated Use '@nx/react/webpack-plugin' instead. */
export const NxReactWebpackPlugin = _NxReactWebpackPlugin;

Expand Down
5 changes: 4 additions & 1 deletion packages/react/plugins/bundle-rollup.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import * as rollup from 'rollup';

// TODO(v20): This should be deprecated and removed in v22.
// TODO(v22): Remove this in Nx 22 and migrate to explicit rollup.config.js files.
/**
* @deprecated Use `withNx` function from `@nx/rollup/with-nx` in your rollup.config.js file instead. Use `nx g @nx/rollup:convert-to-inferred` to generate the rollup.config.js file if it does not exist.
*/
function getRollupOptions(options: rollup.RollupOptions) {
const extraGlobals = {
react: 'React',
Expand Down
2 changes: 1 addition & 1 deletion packages/react/plugins/component-testing/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ function buildTargetWebpack(

return async () => {
customWebpack = await customWebpack;
// TODO(v20): Component testing need to be agnostic of the underlying executor. With Crystal, we're not using `@nx/webpack:webpack` by default.
// TODO(v21): Component testing need to be agnostic of the underlying executor. With Crystal, we're not using `@nx/webpack:webpack` by default.
// We need to decouple CT from the build target of the app, we just care about bundler config (e.g. webpack.config.js).
// The generated setup should support both Webpack and Vite as documented here: https://docs.cypress.io/guides/component-testing/react/overview
// Related issue: https://github.com/nrwl/nx/issues/21546
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export function applyReactConfig(
const svgrOptions =
typeof options.svgr === 'object' ? options.svgr : defaultSvgrOptions;

// TODO(v20): Remove file-loader and use `?react` querystring to differentiate between asset and SVGR.
// TODO(v21): Remove file-loader and use `?react` querystring to differentiate between asset and SVGR.
// It should be:
// use: [{
// test: /\.svg$/i,
Expand Down
2 changes: 1 addition & 1 deletion packages/webpack/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export {
/** @deprecated Use `configurationGenerator` instead. */
export const webpackProjectGenerator = configurationGenerator;

// TODO(v20): Remove this in favor of deep imports in order to load configs faster (150-200ms faster).
// TODO(v21): Remove this in favor of deep imports in order to load configs faster (150-200ms faster).
/** @deprecated Use NxAppWebpackPlugin from `@nx/webpack/app-plugin` instead. */
export const NxWebpackPlugin = NxAppWebpackPlugin;
/** @deprecated Use NxTsconfigPathsWebpackPlugin from `@nx/webpack/tsconfig-paths-plugin` instead. */
Expand Down
2 changes: 1 addition & 1 deletion packages/webpack/src/executors/webpack/schema.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export interface WebpackExecutorOptions {
extractLicenses?: boolean;
fileReplacements?: FileReplacement[];
generatePackageJson?: boolean;
// TODO(v20): Remove this option
// TODO(v21): Remove this option
/** @deprecated set webpackConfig and provide an explicit webpack.config.js file (See: https://nx.dev/recipes/webpack/webpack-config-setup) */
isolatedConfig?: boolean;
standardWebpackConfigFunction?: boolean;
Expand Down

0 comments on commit 81892b5

Please sign in to comment.