fix(app-config-writer): extra leading comma in eslint.config.mjs when .eslintrc.json only extends fiori-tools plugin
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 156
- Forks
- 71
- Avg merge
- 4d 12h
- Merged PRs (30d)
- 45
Description
Description
When running npx --yes @sap-ux/create@latest convert eslint-config --config recommended-for-s4hana on a project whose .eslintrc.json only extends the fiori-tools plugin (no other rules or plugins), the generated eslint.config.mjs contains an invalid leading comma:
import fioriTools from '@sap-ux/eslint-plugin-fiori-tools';
import { defineConfig } from "eslint/config";
export default defineConfig([,
...fioriTools.configs['recommended-for-s4hana'],
]);
Root Cause
Package: @sap-ux/app-config-writer — src/eslint-config/convert.ts
The injectFioriToolsIntoMigratedConfig function always prepends a , before the spread entry:
content =
content.slice(0, lastBracketIndex) +
`,\n ...fioriTools.configs['${config}'],\n` +
content.slice(lastBracketIndex);
When .eslintrc.json only extends plugin:@sap-ux/eslint-plugin-fiori-tools/*, removeFioriToolsFromExistingConfig strips that entry, leaving { "root": true }. The root property has no flat-config equivalent, so @eslint/migrate-config produces an empty array:
import { defineConfig } from "eslint/config";
export default defineConfig([]);
Injecting ,\n ... before ]); into an empty array produces the invalid [, syntax.
Reproducing Input
.eslintrc.json:
{
"extends": "plugin:@sap-ux/eslint-plugin-fiori-tools/defaultJS",
"root": true
}
Fix
Detect an empty array before injecting and omit the leading comma:
const beforeBracket = content.slice(0, lastBracketIndex);
const isEmptyArray = /\[\s*$/.test(beforeBracket);
const separator = isEmptyArray ? '' : ',';
content =
beforeBracket +
`${separator}\n ...fioriTools.configs['${config}'],\n` +
content.slice(lastBracketIndex);
A fix with a regression test is available on branch fix/convert-cmd.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in src/eslint-config/convert.ts, focusing on injectFioriToolsIntoMigratedConfig and the empty-array output from the migration command. Reproduce the command with the provided .eslintrc.json, then run the regression test available on branch fix/convert-cmd; done means the generated eslint.config.mjs is valid for both empty and non-empty arrays.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- eslint, typescript
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100