refactor(eslint-plugin-mobx): rewrite in TypeScript and type flat/legacy configs - #4699
refactor(eslint-plugin-mobx): rewrite in TypeScript and type flat/legacy configs#4699rakleed wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: 4971f60 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Hi @rakleed Thank you for the PR the issue I see here is that we need to maintain JSDoc + a handwritten declaration file. Thoughts on rewriting the eslint plugin to TS? the eslint plugin is already bundled with rollup. Possible plan:
|
There was a problem hiding this comment.
🟡 Changes recommended
Public type dependencies and generated declaration behavior are not packaged or tested correctly, and the implementation contradicts the stated scope.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds typings for the ESLint plugin’s legacy and flat recommended configurations by migrating its implementation to TypeScript and generating declarations during Rollup builds.
Changes:
- Types plugin rules and recommended configurations.
- Replaces JavaScript sources with TypeScript and updates build/test configuration.
- Adds package metadata and release notes for declarations.
File summaries
| File | Description |
|---|---|
.changeset/eslint-plugin-mobx-flat-config-types.md |
Records the typing enhancement. |
package-lock.json |
Locks the ESLint type dependency. |
packages/eslint-plugin-mobx/package.json |
Declares typings and type-check scripts. |
packages/eslint-plugin-mobx/rollup.config.js |
Switches the build to TypeScript. |
packages/eslint-plugin-mobx/tsconfig.json |
Configures source type-checking. |
packages/eslint-plugin-mobx/src/index.ts |
Types and exports plugin configurations. |
packages/eslint-plugin-mobx/src/index.js |
Removes the previous JavaScript entry point. |
packages/eslint-plugin-mobx/src/utils.ts |
Migrates shared utilities to TypeScript. |
packages/eslint-plugin-mobx/src/utils.js |
Removes the JavaScript utilities. |
packages/eslint-plugin-mobx/src/exhaustive-make-observable.ts |
Types the exhaustive rule. |
packages/eslint-plugin-mobx/src/missing-make-observable.ts |
Migrates and types the rule. |
packages/eslint-plugin-mobx/src/missing-make-observable.js |
Removes the JavaScript rule. |
packages/eslint-plugin-mobx/src/missing-observer.ts |
Types the observer rule. |
packages/eslint-plugin-mobx/src/no-anonymous-observer.ts |
Types the anonymous-observer rule. |
packages/eslint-plugin-mobx/src/unconditional-make-observable.ts |
Migrates and types the rule. |
packages/eslint-plugin-mobx/src/unconditional-make-observable.js |
Removes the JavaScript rule. |
packages/eslint-plugin-mobx/__tests__/exhaustive-make-observable.js |
Updates TypeScript source imports. |
packages/eslint-plugin-mobx/__tests__/missing-make-observable.js |
Updates imports and formatting. |
packages/eslint-plugin-mobx/__tests__/missing-observer.js |
Updates the rule import. |
packages/eslint-plugin-mobx/__tests__/no-anonymous-observer.js |
Updates the rule import. |
packages/eslint-plugin-mobx/__tests__/unconditional-make-observable.js |
Updates imports and formatting. |
Review details
Suppressed comments (1)
packages/eslint-plugin-mobx/src/exhaustive-make-observable.ts:121
- “Listen” is the wrong form here; the fields are being “listed.”
- Files reviewed: 20/21 changed files
- Comments generated: 4
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
a721761 to
c4948ff
Compare
|
@rakleed Thank you! could you please use
Same for |
Rewrites the plugin in TypeScript so mobx.flatConfigs.recommended and mobx.configs.recommended are typed when consumed from a TypeScript eslint.config.ts, per @kubk's suggestion on the PR: - source files renamed to .ts - rollup compiles src/index.ts with rollup-plugin-typescript2 (already used by the other packages in this monorepo), with Babel still running afterwards (via the existing .babelrc.js) to downlevel the output the same way the pre-TypeScript build did - declarations are generated into dist/index.d.ts and "types" points there, so the declaration can no longer drift from the implementation the way a hand-written one could - the generated declaration's `export default` is patched to `export =` after the build, to match the actual module.exports = <value> runtime shape regardless of the consumer's esModuleInterop setting - the standalone .d.ts rollup-plugin-typescript2 emits for every compiled module (not just the bundled entry point) is removed after the build, since dist/index.d.ts is fully self-contained - @types/eslint moved to dependencies, since the generated declaration references it and it isn't installed transitively as a devDependency; @types/estree is pinned to ^1.0.0 as a direct dependency too, since @types/eslint's own dependency on it is unconstrained and this repo already has an incompatible 0.0.39 hoisted at the root Closes mobxjs#4545.
c4948ff to
2184c46
Compare

Rewrites
eslint-plugin-mobxin TypeScript somobx.flatConfigs.recommendedandmobx.configs.recommendedare typed when consumed from a TypeScripteslint.config.ts, per @kubk's suggestion:.ts.src/index.tswithrollup-plugin-typescript2(already used by the other packages in this monorepo). Babel still runs afterwards (via the existing.babelrc.js) so the shipped bundle is downleveled the same way it was before this change.dist/index.d.tsand"types"points there, so the declaration can no longer drift from the implementation the way a hand-written one could. The generatedexport defaultis patched toexport =after the build to match the actualmodule.exports = <value>CommonJS runtime shape regardless of the consumer'sesModuleInteropsetting, and the standalone.d.tsrollup-plugin-typescript2emits per compiled module is removed sincedist/index.d.tsis fully self-contained.@types/eslintmoved todependencies(it's referenced by the generated declaration, so it needs to be installed for consumers too), and@types/estreeis pinned to^1.0.0as a direct dependency, since@types/eslint's own dependency on it is unconstrained.Closes #4545.
Code change checklist
/docs. For new functionality, at leastAPI.mdshould be updatednpm -w mobx run test:performance)