-
Notifications
You must be signed in to change notification settings - Fork 559
build(client): generate eslint 9 configs for all packages #25989
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
build(client): generate eslint 9 configs for all packages #25989
Conversation
|
🔗 No broken links found! ✅ Your attention to detail is admirable. linkcheck output |
19a9099 to
04a275c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR migrates the ESLint configuration to ESLint 9 flat config format by:
- Converting the shared ESLint config from JavaScript to TypeScript (.mjs → .mts)
- Enhancing the generation script to support TypeScript config output and shared config detection
- Generating TypeScript-based flat configs for all packages using the enhanced script
Key changes:
- Script now detects and generates shared configs (e.g.,
examples/eslint.config.data.mts) that are imported by child packages - Type-aware rules are properly scoped to non-test TypeScript files only
- Improved parserOptions handling with projectService support and explicit project configuration where needed
- Migration of .eslintignore patterns into flat config format
Reviewed changes
Copilot reviewed 166 out of 166 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/generate-flat-eslint-configs.ts | Enhanced to support TypeScript output, shared config detection, improved rule handling, and .eslintignore migration |
| common/build/eslint-config-fluid/flat.mts | Converted from .mjs to TypeScript with enhanced configuration including global ignores, projectService, test file handling, and React hooks rules |
| common/build/eslint-config-fluid/flat.mjs | Deleted - replaced by .mts version |
| examples/eslint.config.data.mts | Generated shared config for examples directory |
| */eslint.config.mts | Generated per-package TypeScript configs importing from shared flat.mts config |
alexvy86
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only looked at a few of the generated configs; didn't see anything particularly weird, but it was far from an exhaustive review. I still feel ok overall moving forward with this.
The comment for the config.data.mts file I think does need addressing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file seems to have lost the purpose of its original, i.e. exporting the lists of packages in addition to the final config object, so other packages have an easier time overriding the rules but still being able to reference the original package lists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be fixed in latest. This script now exports the package lists, and downstream configs should import them and use them.
| // Global ignores for all configs. | ||
| // In flat config, a config object with only `ignores` is treated as a global ignore pattern. | ||
| // See: https://eslint.org/docs/latest/use/configure/configuration-files#globally-ignoring-files-with-ignores | ||
| const globalIgnores = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Eslint docs suggest using the globalIgnores() helper for this.
Using defineConfig() like most of the Eslint examples would also be nice but seem more complicated here, and I'm not sure if it gives us any extra value if we're already using the correct types (e.g. FlatConfigArray) when defining our configs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My plan is to completely rewrite these as native flat configs once we are on 9. So I'll make a note of this for when we do that part.
Update the generate-flat-eslint-configs script to preserve named exports from .eslintrc.data.cjs files when generating flat configs. This allows child packages to import and extend the individual lists (e.g., importInternalModulesAllowed) rather than just the composed config. The eslint.config.data.mts now exports: - importInternalModulesAllowed: base list of allowed imports - importInternalModulesAllowedForTest: extended list for test files - default: the composed config array This addresses PR feedback about maintaining the original purpose of the shared config file.
Update external-data and webflow eslint configs to import and use the named exports (importInternalModulesAllowed, importInternalModulesAllowedForTest) from eslint.config.data.mts instead of inlining the expanded arrays. This allows these packages to properly extend the base lists while adding their own custom patterns, matching the original behavior of the legacy .eslintrc.cjs configs.
alexvy86
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked at some of the generated configs, and skimmed the script to generate them. No major concerns. Since this has no effect yet and we'll almost certainly know if there are any issues when we swap the packages to Eslint9, I think this is fine to merge and we can fix issues as we discover them.
This change updates the shared eslint 9 config to be TypeScript and updates the script in scripts/generate-flat-eslint-configs.ts to support a
--typescriptflag to output typescript per-package configs instead of ESM. Also adds a--finalizeflag that will do some additional cleanup once packages are fully migrated to eslint 9.Finally, there are lots of fixes to the script to handle ignore entries, inheritance from other configs, etc. The script now produces configs that work in my testing without any modifications. Even if that doesn't hold true once we are truly migrating (see #25932), merging this beforehand will simplify the review of those changes.
All the configs were generated by the script using this command: