[AI] Add per-package tsconfigs and typescript-strict-plugin for typecheck#7019
[AI] Add per-package tsconfigs and typescript-strict-plugin for typecheck#7019MatissJanis merged 10 commits intomasterfrom
Conversation
…heck Co-authored-by: Cursor <cursoragent@cursor.com>
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for actualbudget-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughAdds a dedicated typecheck pipeline step to lage and introduces per-package typecheck scripts and package-level tsconfig files (with strict/noEmit and typescript-strict-plugin in many packages); moves root plugin config into package configs and adds a tsconfig.root.json. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev
participant CLI as "npm/yarn CLI"
participant Lage as "lage"
participant PKG as "Package scripts (typecheck)"
participant TSC as "tsc / tsc-strict"
Dev->>CLI: run `yarn typecheck` (root)
CLI->>Lage: `lage typecheck`
Lage->>PKG: run each package's `typecheck` script
PKG->>TSC: invoke `tsc --noEmit` and `tsc-strict` (or configured plugin)
TSC-->>PKG: report diagnostics
PKG-->>Lage: pass/fail
Lage-->>CLI: aggregate results
CLI-->>Dev: display aggregate status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
packages/api/tsconfig.json (1)
7-7:"*.ts"inincludeis redundant —"**/*.ts"already covers root-level files.
"*.ts"matches only.tsfiles at the package root. Since"**/*.ts"is also present and matches all.tsfiles recursively (including the root),"*.ts"adds nothing.♻️ Proposed cleanup
- "include": ["*.ts", "**/*.ts", "../../packages/loot-core/typings/pegjs.ts"], + "include": ["**/*.ts", "../../packages/loot-core/typings/pegjs.ts"],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/tsconfig.json` at line 7, The "include" array contains a redundant entry: remove the "*.ts" entry so only the recursive pattern "**/*.ts" (and the pegjs typings path "../../packages/loot-core/typings/pegjs.ts") remains; update the include array to drop the literal "*.ts" string to avoid duplication and rely on "**/*.ts" to cover root-level and nested .ts files.packages/crdt/tsconfig.json (1)
4-6:strict: truebypasses per-file@ts-strict-ignoreopt-out used by other packages.All other packages in this PR (
api,loot-core,desktop-client,plugins-service) usetypescript-strict-pluginto enforce strict mode while still permitting the//@ts-strict-ignore`` escape hatch. Using"strict": truedirectly in compiler options eliminates that per-file opt-out mechanism.If
crdtalready has zero strict violations this is acceptable, but for consistency and to preserve the incremental migration path, consider aligning with the plugin approach:♻️ Proposed alignment with other packages
{ "extends": "../../tsconfig.json", "compilerOptions": { "noEmit": true, - "strict": true + "plugins": [{ "name": "typescript-strict-plugin", "paths": ["."] }] },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/crdt/tsconfig.json` around lines 4 - 6, The tsconfig.json currently sets "strict": true which bypasses the per-file `@ts-strict-ignore` opt-out used elsewhere; change the compiler configuration to remove or disable the top-level "strict": true and instead enable strict checking via the typescript-strict-plugin (or mirror the other packages' setup), so that per-file // `@ts-strict-ignore` remains honored; specifically update the tsconfig.json compilerOptions to match the other packages (remove or set strict to false) and add/configure the typescript-strict-plugin in the crdt package to enforce strictness while preserving per-file opt-outs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/api/tsconfig.json`:
- Line 5: In the tsconfig "plugins" entry where the plugin name is
"typescript-strict-plugin", change the plugin option key "path" to "paths" so
the plugin recognizes the directory restriction; specifically update the plugins
object that contains { "name": "typescript-strict-plugin", "path": ["."] } to
use "paths": [...] in the files where it appears (packages/api/tsconfig.json,
packages/loot-core/tsconfig.json, packages/desktop-client/tsconfig.json,
packages/plugins-service/tsconfig.json) so only the intended files are subjected
to strict checking.
In `@packages/desktop-client/package.json`:
- Line 18: Remove the stray dependency change adding "playwright": "playwright"
from the package.json in the desktop-client package (or revert that specific
entry) so the PR only contains per-package typecheck-related changes; if adding
Playwright was intentional, move that change into a separate commit/PR with a
clear message referencing the desktop-client package instead of bundling it
here.
In `@packages/desktop-electron/tsconfig.json`:
- Line 5: The tsconfig plugin entry uses the incorrect option key "path" instead
of "paths" in the plugins array for the typescript-strict-plugin; update the
plugins object (plugins: [{ "name": "typescript-strict-plugin", "path": ["."]
}]) to use "paths" (plugins: [{ "name": "typescript-strict-plugin", "paths":
["."] }]) so the typescript-strict-plugin receives the correct option name.
In `@packages/sync-server/tsconfig.json`:
- Line 7: The tsconfig plugin configuration uses the wrong option name: change
the plugins entry for "typescript-strict-plugin" to use "paths" (plural) instead
of "path" so the directory scoping is applied; locate the "plugins" arrays that
include { "name": "typescript-strict-plugin", "path": [...] } and rename the key
to "paths" in each tsconfig where that plugin is configured to ensure the option
is not ignored.
---
Duplicate comments:
In `@packages/crdt/package.json`:
- Around line 15-16: The package.json in this package has an inconsistent
scripts section compared to component-library; update the "test" and "typecheck"
script entries to match the canonical scripts used by component-library (i.e.,
use the same commands/options and ordering so they are consistent across
packages), by editing the "test" and "typecheck" values in this package.json to
be identical to the component-library equivalents.
In `@packages/desktop-client/tsconfig.json`:
- Line 5: The tsconfig plugin entry uses the incorrect property name "path"
instead of "paths" for the typescript-strict-plugin configuration; update the
plugins array entry for "typescript-strict-plugin" to use "paths" (i.e., change
"path": ["."] to "paths": ["."]) so the plugin receives the correct option
(refer to the "plugins" object and the "typescript-strict-plugin" entry).
In `@packages/loot-core/tsconfig.json`:
- Line 6: The tsconfig "plugins" entry uses the wrong property name "path"
instead of TypeScript's "paths": locate the plugins array entry with { "name":
"typescript-strict-plugin", "path": [...] } and rename the "path" key to "paths"
so it becomes { "name": "typescript-strict-plugin", "paths": [...] } to match
other package tsconfigs and ensure the plugin paths are picked up.
In `@packages/plugins-service/tsconfig.json`:
- Line 14: The tsconfig has a typo in the "plugins" entry: the plugin object
uses the key "path" instead of the correct "paths"; update the plugins array
object to use "paths" (plural) for the typescript-strict-plugin configuration so
TypeScript will recognize the mapping—i.e., locate the "plugins" entry that
contains { "name": "typescript-strict-plugin", "path": ["."] } and change "path"
to "paths".
---
Nitpick comments:
In `@packages/api/tsconfig.json`:
- Line 7: The "include" array contains a redundant entry: remove the "*.ts"
entry so only the recursive pattern "**/*.ts" (and the pegjs typings path
"../../packages/loot-core/typings/pegjs.ts") remains; update the include array
to drop the literal "*.ts" string to avoid duplication and rely on "**/*.ts" to
cover root-level and nested .ts files.
In `@packages/crdt/tsconfig.json`:
- Around line 4-6: The tsconfig.json currently sets "strict": true which
bypasses the per-file `@ts-strict-ignore` opt-out used elsewhere; change the
compiler configuration to remove or disable the top-level "strict": true and
instead enable strict checking via the typescript-strict-plugin (or mirror the
other packages' setup), so that per-file // `@ts-strict-ignore` remains honored;
specifically update the tsconfig.json compilerOptions to match the other
packages (remove or set strict to false) and add/configure the
typescript-strict-plugin in the crdt package to enforce strictness while
preserving per-file opt-outs.
…lugin path key from "path" to "paths" and add reference to process-worker typings in index.electron.ts.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
🧹 Nitpick comments (4)
packages/api/tsconfig.json (1)
7-7: Minor cleanup inincludearray — redundant glob and verbose cross-package pathTwo small issues:
"*.ts"is subsumed by"**/*.ts"(which also matches files in the current directory);"*.ts"can be dropped."../../packages/loot-core/typings/pegjs.ts"navigates to the repo root and back down unnecessarily —"../loot-core/typings/pegjs.ts"is the simpler equivalent path frompackages/api/.♻️ Proposed fix
- "include": ["*.ts", "**/*.ts", "../../packages/loot-core/typings/pegjs.ts"], + "include": ["**/*.ts", "../loot-core/typings/pegjs.ts"],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/tsconfig.json` at line 7, Update the tsconfig.json "include" array to remove the redundant "*.ts" entry and simplify the relative path to the pegjs typing by replacing "../../packages/loot-core/typings/pegjs.ts" with "../loot-core/typings/pegjs.ts"; edit the "include" array in the tsconfig (the array currently containing "*.ts", "**/*.ts", and the long path) so it only contains "**/*.ts" and the simplified "../loot-core/typings/pegjs.ts".packages/loot-core/tsconfig.json (2)
14-20:**/*.test.tsxand**/*.spec.tsxare not excluded.The
excludelist covers**/*.test.tsand**/*.spec.tsbut omits their.tsxvariants. If any.tsxtest files exist inloot-corenow or are added later, they'll be included in the typecheck. Suggest adding them for completeness to match the pattern indesktop-client/tsconfig.json.♻️ Proposed fix
"exclude": [ "node_modules", "build", "lib-dist", "**/*.test.ts", - "**/*.spec.ts" + "**/*.spec.ts", + "**/*.test.tsx", + "**/*.spec.tsx" ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/loot-core/tsconfig.json` around lines 14 - 20, The tsconfig.json's "exclude" array is missing the .tsx test patterns so .tsx test files will be typechecked; update the "exclude" array (the "exclude" key in packages/loot-core/tsconfig.json) to include "**/*.test.tsx" and "**/*.spec.tsx" alongside the existing "**/*.test.ts" and "**/*.spec.ts" entries so .tsx test/spec files are ignored during compilation.
5-6:vite/clientintypesmay pollute server-side type checking.Including
vite/clientin thetypesarray globally injects Vite client-side type augmentations (e.g.,import.meta.env,import.meta.hot) across all files covered by this tsconfig — including loot-core's Node/server-side modules. This could mask incorrect use of browser-only APIs in server-targeted code.If
vite/clienttypes are only needed for specific browser-targeted files, consider restricting them via a/// <reference types="vite/client" />directive in those files instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/loot-core/tsconfig.json` around lines 5 - 6, The global "types" array currently includes "vite/client", which injects Vite client-side type augmentations into all files; remove "vite/client" from the "types" array in tsconfig (leave "vitest/globals" and "node" intact) and instead add per-file client-type needs via the triple-slash directive /// <reference types="vite/client" /> at the top of browser-only modules that need import.meta typings; this ensures server-side Node modules aren't polluted while preserving Vite types where required.packages/crdt/tsconfig.json (1)
5-6:"strict": trueincompilerOptionsis inconsistent with the plugin-based approach used in other packages.This package uses
"strict": truedirectly incompilerOptions, while sibling packages (e.g.,loot-core,desktop-client) configuretypescript-strict-pluginwith"paths": ["."]to get the same enforcement with per-file opt-out support (//@ts-strict-ignore``).TypeScript plugins don't work at compile-time — they will show errors in the IDE but not during compilation. The
tsc-strictCLI is the tool designed to check strict errors at compile time. With"strict": trueincompilerOptionsand no plugin entry, thetsc-strictstep in the typecheck script is effectively redundant (sincetsc --noEmitalready applies global strict mode), and files cannot use//@ts-strict-ignore`` to opt out during migration.If crdt is fully strict-compliant with no planned opt-outs, this is functionally correct. Otherwise, consider aligning with the plugin-based pattern:
♻️ Proposed change to align with plugin-based approach
{ "extends": "../../tsconfig.json", "compilerOptions": { "noEmit": true, - "strict": true + "plugins": [{ "name": "typescript-strict-plugin", "paths": ["."] }] }, "include": ["*.ts", "src/**/*.ts"], "exclude": ["node_modules", "dist", "**/*.test.ts", "**/*.spec.ts"] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/crdt/tsconfig.json` around lines 5 - 6, Replace the direct "strict": true entry in compilerOptions with the plugin-based configuration used in sibling packages: remove the "strict": true boolean and add a compilerOptions.plugins entry that references "typescript-strict-plugin" with "paths": ["."], i.e. add a plugins array containing { "name": "typescript-strict-plugin", "paths": ["." ] }; keep compilerOptions otherwise intact so IDE strict checks remain and the tsc-strict CLI can enforce strictness at compile time while allowing per-file // `@ts-strict-ignore` opt-outs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/desktop-client/package.json`:
- Line 18: The "playwright": "playwright" package.json entry is unrelated to
this per-package typecheck change; remove the "playwright": "playwright" entry
from packages/desktop-client package.json (the key "playwright") so the package
doesn't expose the Playwright CLI here; if Playwright is needed for tests/CI,
add it to the repository-level devDependencies or the appropriate test-related
package's devDependencies instead.
In `@packages/desktop-electron/package.json`:
- Line 12: The "typecheck" npm script references the tsc-strict binary but this
package's devDependencies do not include typescript-strict-plugin; either add
"typescript-strict-plugin" (or the correct package that provides the tsc-strict
CLI) to this package's devDependencies or change the script to invoke the tool
via npx (e.g., "npx tsc-strict") so resolution works; update the package.json
"typecheck" script accordingly and ensure the same dependency change was made as
in packages/sync-server for consistency.
In `@packages/desktop-electron/tsconfig.json`:
- Line 5: The tsconfig includes the IDE-only "typescript-strict-plugin" entry
but the package's CI won't enforce those rules; update the package's npm
"typecheck" script to run the shared tsc-strict helper (same change applied to
plugins-service) so CI runs the strict TypeScript checks—locate the package's
package.json, add or modify the "typecheck" script to invoke tsc-strict (or the
equivalent script used in other packages), and ensure the repo-level tsc-strict
target enforces the "typescript-strict-plugin" behavior for the desktop-electron
package.
---
Nitpick comments:
In `@packages/api/tsconfig.json`:
- Line 7: Update the tsconfig.json "include" array to remove the redundant
"*.ts" entry and simplify the relative path to the pegjs typing by replacing
"../../packages/loot-core/typings/pegjs.ts" with
"../loot-core/typings/pegjs.ts"; edit the "include" array in the tsconfig (the
array currently containing "*.ts", "**/*.ts", and the long path) so it only
contains "**/*.ts" and the simplified "../loot-core/typings/pegjs.ts".
In `@packages/crdt/tsconfig.json`:
- Around line 5-6: Replace the direct "strict": true entry in compilerOptions
with the plugin-based configuration used in sibling packages: remove the
"strict": true boolean and add a compilerOptions.plugins entry that references
"typescript-strict-plugin" with "paths": ["."], i.e. add a plugins array
containing { "name": "typescript-strict-plugin", "paths": ["." ] }; keep
compilerOptions otherwise intact so IDE strict checks remain and the tsc-strict
CLI can enforce strictness at compile time while allowing per-file //
`@ts-strict-ignore` opt-outs.
In `@packages/loot-core/tsconfig.json`:
- Around line 14-20: The tsconfig.json's "exclude" array is missing the .tsx
test patterns so .tsx test files will be typechecked; update the "exclude" array
(the "exclude" key in packages/loot-core/tsconfig.json) to include
"**/*.test.tsx" and "**/*.spec.tsx" alongside the existing "**/*.test.ts" and
"**/*.spec.ts" entries so .tsx test/spec files are ignored during compilation.
- Around line 5-6: The global "types" array currently includes "vite/client",
which injects Vite client-side type augmentations into all files; remove
"vite/client" from the "types" array in tsconfig (leave "vitest/globals" and
"node" intact) and instead add per-file client-type needs via the triple-slash
directive /// <reference types="vite/client" /> at the top of browser-only
modules that need import.meta typings; this ensures server-side Node modules
aren't polluted while preserving Vite types where required.
…dd new process-worker typings file for global Process augmentation.
✅ Deploy Preview for actualbudget-website ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…removing tsconfig.dist.json files and updating build scripts to use default TypeScript compilation. Adjusted compiler options to target ES2021 and enable declaration generation.
…exclude patterns for better file management.
…copied to the correct directory by creating the destination folder if it doesn't exist.
…s to refine include and exclude patterns for improved file management.
…pescript-strict-plugin for enhanced type checking and maintain consistency in package.json files.
d2e1310 to
11cb541
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/desktop-client/package.json`:
- Line 18: The dependency entry "playwright": "playwright" in
packages/desktop-client/package.json is unrelated to the per-package
typecheck/tsconfig changes; remove this dependency line from the current PR or
move it into a separate commit/PR that clearly documents adding Playwright (or
revert if accidental). Locate the "playwright": "playwright" entry in
package.json and either delete it from this diff or extract it into its own
commit/PR so the current change set only contains typecheck/tsconfig-related
edits.
c934b94 to
320a2a4
Compare
Related: #6309
Why do this?
Bundle Stats
View detailed bundle stats
desktop-client
Total
View detailed bundle breakdown
Added
No assets were added
Removed
No assets were removed
Bigger
No assets were bigger
Smaller
No assets were smaller
Unchanged
loot-core
Total
Changeset
home/runner/work/actual/actual/packages/crdt/src/crdt/timestamp.tsView detailed bundle breakdown
Added
Removed
Bigger
No assets were bigger
Smaller
No assets were smaller
Unchanged
No assets were unchanged
api
Total
Changeset
home/runner/work/actual/actual/packages/crdt/src/crdt/timestamp.tsView detailed bundle breakdown
Added
No assets were added
Removed
No assets were removed
Bigger
Smaller
No assets were smaller
Unchanged
No assets were unchanged