Conversation
WalkthroughTests and plugin code were updated to use a dynamically computed ESLint rule key based on TSDOWN_VERSION. The test now stubs the env and dynamically imports the rule module. The plugin exports rules with a computed key. The build config defines the env variable from package.json. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Tester as Test Runner (vitest)
participant Env as vi.stubEnv
participant Mod as migration/index.ts
participant ESLint as RuleTester
Tester->>Env: stub process.env.TSDOWN_VERSION = manifest.version
note over Env: Ensures version available at import time
Tester->>Mod: dynamic import()
Mod->>Mod: compute ruleName from TSDOWN_VERSION (major)
Mod->>Tester: export rules { [ruleName]: theRule }
Tester->>ESLint: tester.run(ruleName, theRule, cases)
ESLint-->>Tester: results (valid/invalid as specified)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
migration/index.spec.ts (1)
13-17: Refactor async setup tobeforeAllinstead of asyncdescribe.Async
describecallbacks are discouraged in testing frameworks. The dynamic import and environment setup should occur inbeforeAllto follow best practices.Apply this diff to refactor:
-describe("Migration", async () => { - vi.stubEnv("TSDOWN_VERSION", manifest.version); - const { default: migration } = await import("./index.ts"); - const ruleName = `v${manifest.version.split(".")[0]}`; - const theRule = migration.rules[ruleName as keyof typeof migration.rules]; +describe("Migration", () => { + let migration: Awaited<ReturnType<typeof import("./index.ts")>>["default"]; + let ruleName: string; + let theRule: ReturnType<typeof import("./index.ts")>["default"]["rules"][string]; + + beforeAll(async () => { + vi.stubEnv("TSDOWN_VERSION", manifest.version); + const imported = await import("./index.ts"); + migration = imported.default; + ruleName = `v${manifest.version.split(".")[0]}`; + theRule = migration.rules[ruleName as keyof typeof migration.rules]; + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
migration/index.spec.ts(1 hunks)migration/index.ts(2 hunks)migration/tsdown.config.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-29T06:00:10.793Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2974
File: express-zod-api/src/zts-helpers.ts:2-3
Timestamp: 2025-09-29T06:00:10.793Z
Learning: tsdown handles module resolution and bundling independently of TypeScript's native compilation settings like rewriteRelativeImportExtensions, making such TypeScript emit-related configuration unnecessary when using tsdown for builds.
Applied to files:
migration/tsdown.config.ts
🔇 Additional comments (2)
migration/tsdown.config.ts (1)
2-2: LGTM! Clean manifest integration.The import attributes syntax and define block are correctly structured. The double-quoted string interpolation ensures tsdown replaces
process.env.TSDOWN_VERSIONwith a proper string literal at build time.Also applies to: 9-11
migration/index.ts (1)
96-96: Dynamic rule export key format validatedruleNamederives frommanifest.version.split('.')[0]andmigration/package.jsonversion matches semver, ensuring validv<number>keys.
RobinTail
added
refactoring
1 participant
Need it for #2980
Summary by CodeRabbit
New Features
Chores
Tests