Skip to content

Commit

Permalink
fix(angular): handle removed angular-eslint rules in root eslint conf…
Browse files Browse the repository at this point in the history 
…ig files and update package (#29262)

<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

<!-- 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 -->

The migration to remove Angular ESLint rules that were removed in v19
does not handle these rules in the root eslint config files.

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

The migration to remove Angular ESLint rules that were removed in v19
should handle these rules in the root eslint config files.

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

Fixes #
  • Loading branch information
@leosvelperez
leosvelperez authored Dec 10, 2024
1 parent 7e38824 commit df77fde
Show file tree
Hide file tree
Showing 7 changed files with 526 additions and 293 deletions.
8 changes: 4 additions & 4 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@
"@angular-devkit/build-angular": "~19.0.0",
"@angular-devkit/core": "~19.0.0",
"@angular-devkit/schematics": "~19.0.0",
"@angular-eslint/eslint-plugin": "19.0.0",
"@angular-eslint/eslint-plugin-template": "19.0.0",
"@angular-eslint/template-parser": "19.0.0",
"@angular-eslint/eslint-plugin": "19.0.2",
"@angular-eslint/eslint-plugin-template": "19.0.2",
"@angular-eslint/template-parser": "19.0.2",
"@angular/cli": "~19.0.0",
"@angular/common": "~19.0.0",
"@angular/compiler": "~19.0.0",
Expand Down Expand Up @@ -154,7 +154,7 @@
"@zkochan/js-yaml": "0.0.7",
"ai": "^2.2.10",
"ajv": "^8.12.0",
"angular-eslint": "19.0.0",
"angular-eslint": "19.0.2",
"autoprefixer": "10.4.13",
"babel-jest": "29.7.0",
"babel-loader": "^9.1.2",
Expand Down
29 changes: 29 additions & 0 deletions packages/angular/migrations.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1368,6 +1368,35 @@
"alwaysAddToPackageJson": false
}
}
},
"20.2.2-angular-eslint": {
"version": "20.2.2-beta.0",
"requires": {
"eslint": "^8.57.0 || ^9.0.0",
"@angular/core": ">= 19.0.0 < 20.0.0"
},
"packages": {
"angular-eslint": {
"version": "^19.0.2",
"alwaysAddToPackageJson": false
},
"@angular-eslint/eslint-plugin": {
"version": "^19.0.2",
"alwaysAddToPackageJson": false
},
"@angular-eslint/eslint-plugin-template": {
"version": "^19.0.2",
"alwaysAddToPackageJson": false
},
"@angular-eslint/template-parser": {
"version": "^19.0.2",
"alwaysAddToPackageJson": false
},
"@angular-eslint/utils": {
"version": "^19.0.2",
"alwaysAddToPackageJson": false
}
}
}
}
}
179 changes: 168 additions & 11 deletions packages/angular/src/migrations/update-20-2-0/remove-angular-eslint-rules.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
type Tree,
} from '@nx/devkit';
import { createTreeWithEmptyWorkspace } from '@nx/devkit/testing';
import migration from './remove-angular-eslint-rules';
import migration, { rulesToRemove } from './remove-angular-eslint-rules';

let projectGraph: ProjectGraph;
jest.mock('@nx/devkit', () => ({
Expand Down Expand Up @@ -46,11 +46,7 @@ describe('remove-angular-eslint-rules', () => {
});

describe('.eslintrc.json', () => {
it.each([
['@angular-eslint/no-host-metadata-property'],
['@angular-eslint/sort-ngmodule-metadata-arrays'],
['@angular-eslint/prefer-standalone-component'],
])('should remove %s rule', async (rule) => {
it.each(rulesToRemove)('should remove %s rule', async (rule) => {
writeJson(tree, 'apps/app1/.eslintrc.json', {
overrides: [
{
Expand Down Expand Up @@ -94,14 +90,88 @@ describe('remove-angular-eslint-rules', () => {
"
`);
});

it('should handle rules set in the root config', async () => {
writeJson(tree, '.eslintrc.json', {
overrides: [
{
files: ['*.ts'],
rules: {
'@angular-eslint/no-host-metadata-property': ['error'],
'@angular-eslint/sort-ngmodule-metadata-arrays': ['error'],
'@angular-eslint/prefer-standalone-component': ['error'],
},
},
],
});
writeJson(tree, 'apps/app1/.eslintrc.json', {
extends: '../../.eslintrc.json',
});

await migration(tree);

expect(tree.read('.eslintrc.json', 'utf8')).toMatchInlineSnapshot(`
"{
"overrides": [
{
"files": ["*.ts"],
"rules": {}
}
]
}
"
`);
expect(tree.read('apps/app1/.eslintrc.json', 'utf8'))
.toMatchInlineSnapshot(`
"{
"extends": "../../.eslintrc.json"
}
"
`);
});

it('should handle rules set in the root base config', async () => {
writeJson(tree, '.eslintrc.base.json', {
overrides: [
{
files: ['*.ts'],
rules: {
'@angular-eslint/no-host-metadata-property': ['error'],
'@angular-eslint/sort-ngmodule-metadata-arrays': ['error'],
'@angular-eslint/prefer-standalone-component': ['error'],
},
},
],
});
writeJson(tree, 'apps/app1/.eslintrc.json', {
extends: '../../.eslintrc.base.json',
});

await migration(tree);

expect(tree.read('.eslintrc.base.json', 'utf8')).toMatchInlineSnapshot(`
"{
"overrides": [
{
"files": ["*.ts"],
"rules": {}
}
]
}
"
`);
expect(tree.read('apps/app1/.eslintrc.json', 'utf8'))
.toMatchInlineSnapshot(`
"{
"extends": "../../.eslintrc.base.json"
}
"
`);
});
});

describe('flat config', () => {
it.each([
['@angular-eslint/no-host-metadata-property'],
['@angular-eslint/sort-ngmodule-metadata-arrays'],
['@angular-eslint/prefer-standalone-component'],
])('should remove %s rule', async (rule) => {
it.each(rulesToRemove)('should remove %s rule', async (rule) => {
tree.write('eslint.config.js', 'module.exports = [];');
tree.write(
'apps/app1/eslint.config.js',
Expand Down Expand Up @@ -151,5 +221,92 @@ describe('remove-angular-eslint-rules', () => {
"
`);
});

it('should handle rules set in the root config', async () => {
tree.write(
'eslint.config.js',
`module.exports = [
{
files: ['*.ts'],
rules: {
'@angular-eslint/no-host-metadata-property': ['error'],
'@angular-eslint/sort-ngmodule-metadata-arrays': ['error'],
'@angular-eslint/prefer-standalone-component': ['error'],
},
},
];
`
);
tree.write(
'apps/app1/eslint.config.js',
`const baseConfig = require('../../eslint.config.js');
module.exports = [...baseConfig];
`
);

await migration(tree);

expect(tree.read('eslint.config.js', 'utf8')).toMatchInlineSnapshot(`
"module.exports = [
{
files: ['**/*.ts'],
rules: {},
},
];
"
`);
expect(tree.read('apps/app1/eslint.config.js', 'utf8'))
.toMatchInlineSnapshot(`
"const baseConfig = require('../../eslint.config.js');
module.exports = [...baseConfig];
"
`);
});

it('should handle rules set in the root base config', async () => {
tree.write(
'eslint.base.config.js',
`module.exports = [
{
files: ['*.ts'],
rules: {
'@angular-eslint/no-host-metadata-property': ['error'],
'@angular-eslint/sort-ngmodule-metadata-arrays': ['error'],
'@angular-eslint/prefer-standalone-component': ['error'],
},
},
];
`
);
tree.write('eslint.config.js', 'module.exports = [];');
tree.write(
'apps/app1/eslint.config.js',
`const baseConfig = require('../../eslint.base.config.js');
module.exports = [...baseConfig];
`
);

await migration(tree);

expect(tree.read('eslint.base.config.js', 'utf8')).toMatchInlineSnapshot(`
"module.exports = [
{
files: ['**/*.ts'],
rules: {},
},
];
"
`);
expect(tree.read('apps/app1/eslint.config.js', 'utf8'))
.toMatchInlineSnapshot(`
"const baseConfig = require('../../eslint.base.config.js');
module.exports = [...baseConfig];
"
`);
});
});
});
64 changes: 49 additions & 15 deletions packages/angular/src/migrations/update-20-2-0/remove-angular-eslint-rules.ts
View file
Original file line number Diff line number Diff line change
@@ -1,16 +1,24 @@
import { formatFiles, type Tree } from '@nx/devkit';
import {
findEslintFile,
isEslintConfigSupported,
lintConfigHasOverride,
updateOverrideInLintConfig,
} from '@nx/eslint/src/generators/utils/eslint-file';
import { getProjectsFilteredByDependencies } from '../utils/projects';

export const rulesToRemove = [
'@angular-eslint/no-host-metadata-property',
'@angular-eslint/sort-ngmodule-metadata-arrays',
'@angular-eslint/prefer-standalone-component',
];

export default async function (tree: Tree) {
const projects = await getProjectsFilteredByDependencies(tree, [
'npm:@angular/core',
]);

let hasRootProject = false;
for (const {
project: { root },
} of projects) {
Expand All @@ -19,25 +27,51 @@ export default async function (tree: Tree) {
continue;
}

removeRule(tree, root, '@angular-eslint/no-host-metadata-property');
removeRule(tree, root, '@angular-eslint/sort-ngmodule-metadata-arrays');
removeRule(tree, root, '@angular-eslint/prefer-standalone-component');
if (root === '.') {
hasRootProject = true;
}

removeRules(tree, root);
}

/**
* We need to handle both a root config file (e.g. eslint.config.js) and a
* potential base config file (e.g. eslint.base.config.js). We can't use
* `findEslintFile` because it would return only one or the other depending
* on whether a root is provided and the existence of the files. So, we
* handle each of them separately.
*/

// check root config, provide a root so it doesn't try to lookup a base config
if (!hasRootProject) {
// if there is no root project the root eslint config has not been processed
if (isEslintConfigSupported(tree, '')) {
removeRules(tree, '');
}
}

// handle root base config, not providing a root will prioritize a base config
const baseEslintConfig = findEslintFile(tree);
if (baseEslintConfig && baseEslintConfig.includes('.base.')) {
removeRules(tree, baseEslintConfig);
}

await formatFiles(tree);
}

function removeRule(tree: Tree, root: string, rule: string) {
const lookup: Parameters<typeof lintConfigHasOverride>[2] = (o) =>
!!o.rules?.[rule];
if (!lintConfigHasOverride(tree, root, lookup, true)) {
// it's not using the rule, skip
return;
}
function removeRules(tree: Tree, root: string): void {
for (const rule of rulesToRemove) {
const lookup: Parameters<typeof lintConfigHasOverride>[2] = (o) =>
!!o.rules?.[rule];
if (!lintConfigHasOverride(tree, root, lookup)) {
// it's not using the rule, skip
continue;
}

// there is an override containing the rule, remove the rule
updateOverrideInLintConfig(tree, root, lookup, (o) => {
delete o.rules[rule];
return o;
});
// there is an override containing the rule, remove the rule
updateOverrideInLintConfig(tree, root, lookup, (o) => {
delete o.rules[rule];
return o;
});
}
}
2 changes: 1 addition & 1 deletion packages/angular/src/utils/versions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export const browserSyncVersion = '^3.0.0';
export const moduleFederationNodeVersion = '~2.6.11';
export const moduleFederationEnhancedVersion = '0.7.6';

export const angularEslintVersion = '^19.0.0';
export const angularEslintVersion = '^19.0.2';
export const typescriptEslintVersion = '^7.16.0';
export const tailwindVersion = '^3.0.2';
export const postcssVersion = '^8.4.5';
Expand Down
Loading

0 comments on commit df77fde

Please sign in to comment.