SAP / SAP/open-ux-tools

fix(app-config-writer): extra leading comma in eslint.config.mjs when .eslintrc.json only extends fiori-tools plugin

Open Beginner friendly
#4,792 0 comments 0 reactions 0 assignees View on GitHub

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-writersrc/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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.