Skip to content

Commit 08dcdc1

Browse files
jaysooclaude
andcommitted
fix(linter): handle variable references in replaceOverride
The migration generator (@nx/plugin:migration) fails when the project's ESLint flat config contains variable references in property values, such as: `plugins: { 'package-json': pluginPackageJson }` This happens because `replaceOverride` uses `parseTextToJson` to parse the config, which fails for non-JSON-serializable JavaScript expressions. The migration generator should work correctly even when the ESLint config contains variable references. The fix applies the same approach used in PR #33548 for `hasOverride`: using AST-based property extraction instead of JSON parsing. Fixes #34010 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 0119275 commit 08dcdc1

File tree

3 files changed

+272
-72
lines changed

3 files changed

+272
-72
lines changed

packages/eslint/src/generators/utils/flat-config/ast-utils.spec.ts

Lines changed: 142 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -681,22 +681,22 @@ export default [
681681
682682
export default [
683683
{
684-
"files": [
685-
"my-lib/**/*.ts",
686-
"my-lib/**/*.tsx"
687-
],
688-
"rules": {
689-
"my-rule": "error"
690-
}
684+
files: [
685+
"my-lib/**/*.ts",
686+
"my-lib/**/*.tsx"
687+
],
688+
rules: {
689+
"my-rule": "error"
690+
}
691691
},
692692
{
693-
"files": [
694-
"my-lib/**/*.ts",
695-
"my-lib/**/*.js"
696-
],
697-
"rules": {
698-
"my-rule": "error"
699-
}
693+
files: [
694+
"my-lib/**/*.ts",
695+
"my-lib/**/*.js"
696+
],
697+
rules: {
698+
"my-rule": "error"
699+
}
700700
},
701701
{
702702
files: [
@@ -752,14 +752,14 @@ export default [
752752
753753
export default [
754754
{
755-
"files": [
756-
"my-lib/**/*.ts",
757-
"my-lib/**/*.tsx"
758-
],
759-
"rules": {
760-
"my-ts-rule": "error",
761-
"my-new-rule": "error"
762-
}
755+
files: [
756+
"my-lib/**/*.ts",
757+
"my-lib/**/*.tsx"
758+
],
759+
rules: {
760+
"my-ts-rule": "error",
761+
"my-new-rule": "error"
762+
}
763763
},
764764
{
765765
files: [
@@ -808,18 +808,132 @@ export default [
808808
export default [
809809
...compat.config({ extends: ["plugin:@nx/typescript"] }).map(config => ({
810810
...config,
811-
"files": [
811+
files: [
812812
"my-lib/**/*.ts",
813813
"my-lib/**/*.tsx"
814-
],
815-
"rules": {
816-
"my-ts-rule": "error",
817-
"my-new-rule": "error"
818-
}
814+
],
815+
rules: {
816+
"my-ts-rule": "error",
817+
"my-new-rule": "error"
818+
}
819819
}),
820820
];"
821821
`);
822822
});
823+
824+
it('should update files while preserving variable references (ESM)', () => {
825+
const content = `
826+
import pluginPackageJson from "eslint-plugin-package-json";
827+
import jsoncParser from "jsonc-eslint-parser";
828+
829+
export default [
830+
{
831+
files: ["package.json"],
832+
plugins: { "package-json": pluginPackageJson },
833+
languageOptions: {
834+
parser: jsoncParser,
835+
},
836+
rules: {
837+
'@nx/nx-plugin-checks': 'error'
838+
}
839+
}
840+
];`;
841+
842+
const result = replaceOverride(
843+
content,
844+
'',
845+
(o) =>
846+
Object.keys(o.rules ?? {}).includes('@nx/nx-plugin-checks') ||
847+
Object.keys(o.rules ?? {}).includes('@nrwl/nx/nx-plugin-checks'),
848+
(o) => {
849+
const files = Array.isArray(o.files) ? o.files : [o.files];
850+
return {
851+
...o,
852+
files: [...files, './migrations.json'],
853+
};
854+
}
855+
);
856+
857+
// Property-level AST update: only files is modified, plugins with variable refs is preserved
858+
expect(result).toMatchInlineSnapshot(`
859+
"
860+
import pluginPackageJson from "eslint-plugin-package-json";
861+
import jsoncParser from "jsonc-eslint-parser";
862+
863+
export default [
864+
{
865+
files: [
866+
"package.json",
867+
"./migrations.json"
868+
],
869+
plugins: { "package-json": pluginPackageJson },
870+
languageOptions: {
871+
parser: jsoncParser,
872+
},
873+
rules: {
874+
'@nx/nx-plugin-checks': 'error'
875+
}
876+
}
877+
];"
878+
`);
879+
});
880+
881+
it('should update files while preserving variable references (CJS)', () => {
882+
const content = `
883+
const pluginPackageJson = require("eslint-plugin-package-json");
884+
const jsoncParser = require("jsonc-eslint-parser");
885+
886+
module.exports = [
887+
{
888+
files: ["package.json"],
889+
plugins: { "package-json": pluginPackageJson },
890+
languageOptions: {
891+
parser: jsoncParser,
892+
},
893+
rules: {
894+
'@nx/nx-plugin-checks': 'error'
895+
}
896+
}
897+
];`;
898+
899+
const result = replaceOverride(
900+
content,
901+
'',
902+
(o) =>
903+
Object.keys(o.rules ?? {}).includes('@nx/nx-plugin-checks') ||
904+
Object.keys(o.rules ?? {}).includes('@nrwl/nx/nx-plugin-checks'),
905+
(o) => {
906+
const files = Array.isArray(o.files) ? o.files : [o.files];
907+
return {
908+
...o,
909+
files: [...files, './migrations.json'],
910+
};
911+
}
912+
);
913+
914+
// Property-level AST update: only files is modified, plugins with variable refs is preserved
915+
expect(result).toMatchInlineSnapshot(`
916+
"
917+
const pluginPackageJson = require("eslint-plugin-package-json");
918+
const jsoncParser = require("jsonc-eslint-parser");
919+
920+
module.exports = [
921+
{
922+
files: [
923+
"package.json",
924+
"./migrations.json"
925+
],
926+
plugins: { "package-json": pluginPackageJson },
927+
languageOptions: {
928+
parser: jsoncParser,
929+
},
930+
rules: {
931+
'@nx/nx-plugin-checks': 'error'
932+
}
933+
}
934+
];"
935+
`);
936+
});
823937
});
824938

825939
describe('removePlugin', () => {

packages/eslint/src/generators/utils/flat-config/ast-utils.ts

Lines changed: 129 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,66 @@ function extractPropertiesFromObjectLiteral(
295295
}
296296

297297
/**
298-
* Finds an override matching the lookup function and applies the update function to it
298+
* Find a property assignment node by name in an object literal.
299+
*/
300+
function findPropertyNode(
301+
node: ts.ObjectLiteralExpression,
302+
propertyName: string
303+
): ts.PropertyAssignment | undefined {
304+
for (const prop of node.properties) {
305+
if (ts.isPropertyAssignment(prop)) {
306+
const name = prop.name.getText().replace(/['"]/g, '');
307+
if (name === propertyName) {
308+
return prop;
309+
}
310+
}
311+
}
312+
return undefined;
313+
}
314+
315+
/**
316+
* Find properties that are added, changed, or removed.
317+
*/
318+
function findChangedProperties(
319+
original: Record<string, unknown>,
320+
updated: Record<string, unknown>
321+
): string[] {
322+
const changed: string[] = [];
323+
// Check modified/added properties
324+
for (const key of Object.keys(updated)) {
325+
if (JSON.stringify(original[key]) !== JSON.stringify(updated[key])) {
326+
changed.push(key);
327+
}
328+
}
329+
// Check removed properties
330+
for (const key of Object.keys(original)) {
331+
if (!(key in updated)) {
332+
changed.push(key);
333+
}
334+
}
335+
return changed;
336+
}
337+
338+
/**
339+
* Serialize a value to JavaScript source code. Handles converting eslintrc parser format to flat config format.
340+
*/
341+
function serializeValue(value: unknown, format: 'mjs' | 'cjs'): string {
342+
const parserReplacement =
343+
format === 'mjs'
344+
? (parser: string) => `(await import('${parser}'))`
345+
: (parser: string) => `require('${parser}')`;
346+
347+
return JSON.stringify(value, null, 2)
348+
.replace(
349+
/"parser": "([^"]+)"/g,
350+
(_, parser) => `"parser": ${parserReplacement(parser)}`
351+
)
352+
.replaceAll(/\n/g, '\n '); // Maintain indentation
353+
}
354+
355+
/**
356+
* Finds an override matching the lookup function and applies the update function to it.
357+
* Uses property-level AST updates to preserve properties with variable references.
299358
*/
300359
export function replaceOverride(
301360
content: string,
@@ -320,52 +379,78 @@ export function replaceOverride(
320379
}
321380
const changes: StringChange[] = [];
322381
exportsArray.forEach((node) => {
323-
if (isOverride(node)) {
324-
let objSource;
325-
let start, end;
326-
if (ts.isObjectLiteralExpression(node)) {
327-
objSource = node.getFullText();
328-
start = node.properties.pos + 1; // keep leading line break
329-
end = node.properties.end;
382+
if (!isOverride(node)) {
383+
return;
384+
}
385+
386+
let objectLiteralNode: ts.ObjectLiteralExpression;
387+
if (ts.isObjectLiteralExpression(node)) {
388+
objectLiteralNode = node;
389+
} else {
390+
// Handle compat.config(...).map(...) pattern
391+
const arrowBody = node['expression'].arguments[0].body.expression;
392+
if (ts.isObjectLiteralExpression(arrowBody)) {
393+
objectLiteralNode = arrowBody;
330394
} else {
331-
const fullNodeText =
332-
node['expression'].arguments[0].body.expression.getFullText();
333-
// strip any spread elements
334-
objSource = fullNodeText.replace(SPREAD_ELEMENTS_REGEXP, '');
335-
start =
336-
node['expression'].arguments[0].body.expression.properties.pos +
337-
(fullNodeText.length - objSource.length);
338-
end = node['expression'].arguments[0].body.expression.properties.end;
395+
return;
339396
}
340-
const data = parseTextToJson(objSource);
341-
if (lookup(data)) {
342-
changes.push({
343-
type: ChangeType.Delete,
344-
start,
345-
length: end - start,
346-
});
347-
let updatedData = update(data);
348-
if (updatedData) {
349-
updatedData = mapFilePaths(updatedData);
350-
351-
const parserReplacement =
352-
format === 'mjs'
353-
? (parser: string) => `(await import('${parser}'))`
354-
: (parser: string) => `require('${parser}')`;
397+
}
355398

356-
changes.push({
357-
type: ChangeType.Insert,
358-
index: start,
359-
text:
360-
' ' +
361-
JSON.stringify(updatedData, null, 2)
362-
.replace(
363-
/"parser": "([^"]+)"/g,
364-
(_, parser) => `"parser": ${parserReplacement(parser)}`
365-
)
366-
.slice(2, -2) // Remove curly braces and start/end line breaks
367-
.replaceAll(/\n/g, '\n '), // Maintain indentation
368-
});
399+
// Use AST-based extraction to handle variable references (e.g., plugins: { 'abc': abc })
400+
const data = extractPropertiesFromObjectLiteral(objectLiteralNode);
401+
if (lookup(data as Linter.ConfigOverride<Linter.RulesRecord>)) {
402+
// Deep clone before update (update functions may mutate nested objects)
403+
const originalData = structuredClone(data);
404+
405+
let updatedData = update?.(data);
406+
if (updatedData) {
407+
updatedData = mapFilePaths(updatedData);
408+
409+
const changedProps = findChangedProperties(
410+
originalData as Record<string, unknown>,
411+
updatedData as Record<string, unknown>
412+
);
413+
414+
for (const propName of changedProps) {
415+
const originalNode = findPropertyNode(objectLiteralNode, propName);
416+
const updatedValue = updatedData[propName];
417+
418+
if (originalNode && !(propName in updatedData)) {
419+
// Delete property that was removed
420+
changes.push({
421+
type: ChangeType.Delete,
422+
start: originalNode.pos,
423+
length: originalNode.end - originalNode.pos,
424+
});
425+
} else if (originalNode) {
426+
// Replace existing property value
427+
const valueNode = originalNode.initializer;
428+
changes.push({
429+
type: ChangeType.Delete,
430+
start: valueNode.pos,
431+
length: valueNode.end - valueNode.pos,
432+
});
433+
changes.push({
434+
type: ChangeType.Insert,
435+
index: valueNode.pos,
436+
text: ' ' + serializeValue(updatedValue, format),
437+
});
438+
} else {
439+
// Add new property at the end of the object
440+
const lastProp =
441+
objectLiteralNode.properties[
442+
objectLiteralNode.properties.length - 1
443+
];
444+
const insertPos = lastProp
445+
? lastProp.end
446+
: objectLiteralNode.pos + 1;
447+
const needsComma = lastProp ? ',' : '';
448+
changes.push({
449+
type: ChangeType.Insert,
450+
index: insertPos,
451+
text: `${needsComma}\n "${propName}": ${serializeValue(updatedValue, format)}`,
452+
});
453+
}
369454
}
370455
}
371456
}

packages/next/src/generators/application/application.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -830,6 +830,7 @@ describe('app', () => {
830830
'**/*.cjs',
831831
'**/*.mjs',
832832
],
833+
// Override or add rules here
833834
rules: {
834835
'@next/next/no-html-link-for-pages': ['error', './pages'],
835836
},

0 commit comments

Comments
 (0)