Skip to content

Conversation

@RobinTail
Copy link
Owner

@RobinTail RobinTail commented Aug 15, 2025

Due to egoist/tsup#1332

Migration guide: https://tsdown.dev/guide/migrate-from-tsup

Findings:

  • removes augmentation from DTS if it's not a .d.ts file — addressed by renaming addressed using banner and extra entrypoint
  • does not support imports without extension in config file
  • does not support importing JSON files without import attributes in config file — addressed by readFile
  • surprisingly keeps the import of a runtime dependency in DTS even if unused (had to patch tsup for that)
  • dts, cleanup and node protocol — are by default
  • attw is integrated when installed

Summary by CodeRabbit

  • Chores
    • Switched the project to a new build tool and added corresponding build configurations; removed legacy bundler configs, patches, and related build scripts; bumped package versions and cleaned build-related dependencies.
  • Tests
    • Updated tests for the new build tooling and added DTS compatibility checks for TypeScript declarations.
  • Notes
    • No user-facing API or runtime behavior changes expected.

@RobinTail RobinTail added dependencies Pull requests that update a dependency file CI/CD labels Aug 15, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 15, 2025

Walkthrough

Repository-wide migration from tsup to tsdown: added tsdown configs, removed tsup configs and shared base/patch, updated package.json scripts/deps, renamed TSUP_* env vars to TSDOWN_* in code/tests, removed a runtime augmentation import, and added DTS compatibility tests; no runtime API signature changes.

Changes

Cohort / File(s) Summary
New tsdown configs
express-zod-api/tsdown.config.ts, migration/tsdown.config.ts, zod-plugin/tsdown.config.ts
Add tsdown configuration modules exporting default config(s) (entries, attw/profile, define/banner). express-zod-api/tsdown.config.ts reads package.json version via top-level await to define TSDOWN_BUILD/TSDOWN_STATIC.
Removed tsup configs / shared base
express-zod-api/tsup.config.ts, migration/tsup.config.ts, zod-plugin/tsup.config.ts, tsup.base.ts
Remove existing tsup configuration files and the shared commons export; eliminate tsup-based build configuration and exports.
Package manifests & scripts
package.json, express-zod-api/package.json, zod-plugin/package.json, migration/package.json
Replace tsup with tsdown in devDependencies and build scripts; remove postbuild packaging steps and adjust related dev deps.
Env var renames in code
express-zod-api/src/common-helpers.ts, express-zod-api/src/server.ts
Swap environment variable references from TSUP_STATIC/TSUP_BUILD to TSDOWN_STATIC/TSDOWN_BUILD (memoization key and debug log).
Tests updated
express-zod-api/tests/* (builtin-logger.spec.ts, last-resort.spec.ts, result-helpers.spec.ts, system.spec.ts)
Replace TSUP_STATIC stubs with TSDOWN_STATIC in multiple specs; assertions unchanged.
Patch file modified & workspace cleanup
patches/tsup.patch, pnpm-workspace.yaml
patches/tsup.patch made treeshake.moduleSideEffects configurable and propagates treeshake; pnpm-workspace.yaml removed the tsup patch entry from patchedDependencies.
zod-plugin import change
zod-plugin/index.ts
Remove the runtime side-effect import ./augmentation; named exports (pack, unpack, getBrand) unchanged.
New compat tests
compat-test/dts.spec.ts
Add Vitest DTS compatibility tests asserting declaration wiring: framework → zod-plugin → augmentation .d.ts files.
Changelogs
CHANGELOG.md, zod-plugin/CHANGELOG.md
Add version entries noting the bundler switch from tsup to tsdown.

Sequence Diagram(s)

(omitted — changes are configuration/tooling and small env/test edits; no control-flow feature to diagram)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

I nibbled tsup leaves and found tsdown's tune,
Env flags hopped over, renamed by moon.
Tests twitch their whiskers, declarations stay neat,
Exports hum softly — new build steps take seat. 🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 94dd750 and 8eb9a6c.

📒 Files selected for processing (1)
  • express-zod-api/package.json (2 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2428
File: express-zod-api/src/index.ts:44-44
Timestamp: 2025-05-28T18:58:10.064Z
Learning: The type-only import `import type {} from "qs";` in express-zod-api/src/index.ts is necessary to avoid TS2742 errors for exported functions like attachRouting, makeRequestMock, testEndpoint, and testMiddleware that have types depending on types/qs. This import provides the reference TypeScript needs to infer portable type names.
📚 Learning: 2025-06-02T21:11:20.768Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2697
File: CHANGELOG.md:5-5
Timestamp: 2025-06-02T21:11:20.768Z
Learning: In the express-zod-api repository, RobinTail follows a release workflow where package.json version is only updated on the master branch after merging all planned release changes. Changelog entries may show future version numbers while package.json remains at the previous version during feature development, and this is intentional workflow, not a version inconsistency that needs to be flagged.

Applied to files:

  • express-zod-api/package.json
📚 Learning: 2025-05-27T20:03:34.213Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: example/factories.ts:35-42
Timestamp: 2025-05-27T20:03:34.213Z
Learning: The `./example` directory in the express-zod-api repository contains demonstration code for educational purposes only, not intended for production use. Example code can make simplified assumptions for brevity and clarity, and should not be flagged for missing production-level error handling, security measures, or edge case handling.

Applied to files:

  • express-zod-api/package.json
📚 Learning: 2025-08-05T14:43:24.702Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2869
File: zod-plugin/package.json:42-44
Timestamp: 2025-08-05T14:43:24.702Z
Learning: Zod version 4 has been officially released and is stable. As of August 2025, the latest published version is 4.0.14 (published July 30, 2025). Zod v4 is available on npm and can be used in peer dependencies with version constraints like "^4.0.0".

Applied to files:

  • express-zod-api/package.json
📚 Learning: 2025-05-27T19:35:57.357Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/tests/buffer-schema.spec.ts:32-37
Timestamp: 2025-05-27T19:35:57.357Z
Learning: In the express-zod-api project, tests are run from the `express-zod-api` workspace directory, and the project uses an ESM-first environment without `__dirname`. Relative paths like `../logo.svg` in test files correctly resolve to the repository root due to this test execution context.

Applied to files:

  • express-zod-api/package.json
📚 Learning: 2025-05-27T20:27:17.015Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/src/json-schema-helpers.ts:1-3
Timestamp: 2025-05-27T20:27:17.015Z
Learning: Ramda is correctly listed as a dependency in express-zod-api/package.json, so imports of ramda utilities are properly supported.

Applied to files:

  • express-zod-api/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: publish
🔇 Additional comments (4)
express-zod-api/package.json (4)

3-3: Pre-release version bump looks good

The move to 25.3.0-beta.1 aligns with the migration scope. Not flagging any workflow concerns; aware of your release practice of updating final versions on master after merge.


51-62: Peer dep on zod v4 looks correct

Targeting zod ^4.0.0 matches current ecosystem stability for v4.


19-25: Verify tsup references removed, tsdown config in place, and prepack paths valid

All checks look clean:

  • No lingering tsup or TSUP_* usages in source files (only mentions in CHANGELOG.md, which is expected).
  • express-zod-api/tsdown.config.ts exists alongside the other packages.
  • cp ../*.md ../LICENSE ./ in prepack will successfully copy all root .md files and LICENSE.

Good to publish as-is.


41-43: Ensure TS target aligns with Node ESM engines
Your engines range (^20.19.0 | ^22.12.0 | ^24.0.0) suits modern ESM. However, all tsconfig.json files set "module": "ES2022" but omit an explicit "target" (defaulting to ES5), and tsdown.config.ts doesn’t specify target or format. To avoid accidental down-leveling:

  • express-zod-api/tsconfig.json: add
    "compilerOptions": {
      "module": "ES2022",
      "target": "ES2022",  // or "ESNext"
      "moduleResolution": "Bundler",
      
    }
  • express-zod-api/tsdown.config.ts: include a target or format option in defineConfig so the output matches your Node engine range.

[optional_refactors_recommended]

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch use-tsdown

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@socket-security
Copy link

socket-security bot commented Aug 15, 2025

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedtsdown@​0.14.1921008796100

View full report

@coveralls-official
Copy link

coveralls-official bot commented Aug 15, 2025

Coverage Status

coverage: 100.0%. remained the same
when pulling 47ad912 on use-tsdown
into d8c2fd7 on master.

@RobinTail RobinTail added the external bug it's a bug, but in a dependency label Aug 15, 2025
@RobinTail RobinTail added the patched Some dependency is patched label Aug 15, 2025
@RobinTail RobinTail added the refactoring The better way to achieve the same result label Aug 15, 2025
@RobinTail RobinTail marked this pull request as ready for review August 15, 2025 21:03
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
express-zod-api/tsdown.config.ts (1)

4-4: Make the package.json read robust to working directory differences.

Using "./package.json" relies on the process CWD. Resolve relative to the config file to avoid breakage when run from the repo root or via different tooling.

Apply:

-const { version } = JSON.parse(await readFile("./package.json", "utf8"));
+const { version } = JSON.parse(
+  await readFile(new URL("./package.json", import.meta.url), "utf8"),
+);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between d8c2fd7 and 64aca86.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (20)
  • express-zod-api/package.json (1 hunks)
  • express-zod-api/src/common-helpers.ts (1 hunks)
  • express-zod-api/src/server.ts (1 hunks)
  • express-zod-api/tests/builtin-logger.spec.ts (1 hunks)
  • express-zod-api/tests/last-resort.spec.ts (1 hunks)
  • express-zod-api/tests/result-helpers.spec.ts (1 hunks)
  • express-zod-api/tests/system.spec.ts (1 hunks)
  • express-zod-api/tsdown.config.ts (1 hunks)
  • express-zod-api/tsup.config.ts (0 hunks)
  • migration/package.json (1 hunks)
  • migration/tsdown.config.ts (1 hunks)
  • migration/tsup.config.ts (0 hunks)
  • package.json (1 hunks)
  • patches/tsup.patch (0 hunks)
  • pnpm-workspace.yaml (0 hunks)
  • tsup.base.ts (0 hunks)
  • zod-plugin/index.ts (1 hunks)
  • zod-plugin/package.json (1 hunks)
  • zod-plugin/tsdown.config.ts (1 hunks)
  • zod-plugin/tsup.config.ts (0 hunks)
💤 Files with no reviewable changes (6)
  • pnpm-workspace.yaml
  • migration/tsup.config.ts
  • patches/tsup.patch
  • express-zod-api/tsup.config.ts
  • zod-plugin/tsup.config.ts
  • tsup.base.ts
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-06-14T16:42:52.972Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2736
File: express-zod-api/tsup.config.ts:12-26
Timestamp: 2025-06-14T16:42:52.972Z
Learning: In express-zod-api tsup configurations, the direct mutation of `options.supported` in the `esbuildOptions` callback is intentional behavior and should not be flagged as a side effect issue.

Applied to files:

  • express-zod-api/tests/last-resort.spec.ts
  • express-zod-api/tests/builtin-logger.spec.ts
  • express-zod-api/src/server.ts
  • express-zod-api/tests/system.spec.ts
  • express-zod-api/tests/result-helpers.spec.ts
  • zod-plugin/package.json
  • express-zod-api/tsdown.config.ts
  • express-zod-api/package.json
  • express-zod-api/src/common-helpers.ts
📚 Learning: 2025-05-27T19:35:57.357Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/tests/buffer-schema.spec.ts:32-37
Timestamp: 2025-05-27T19:35:57.357Z
Learning: In the express-zod-api project, tests are run from the `express-zod-api` workspace directory, and the project uses an ESM-first environment without `__dirname`. Relative paths like `../logo.svg` in test files correctly resolve to the repository root due to this test execution context.

Applied to files:

  • express-zod-api/tests/last-resort.spec.ts
  • express-zod-api/tests/system.spec.ts
  • express-zod-api/tests/result-helpers.spec.ts
  • express-zod-api/package.json
📚 Learning: 2025-06-02T21:08:56.475Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2697
File: CHANGELOG.md:5-5
Timestamp: 2025-06-02T21:08:56.475Z
Learning: The `cjs-test` directory in the express-zod-api repository is a test workspace and should be excluded when checking for main project version consistency with changelog entries.

Applied to files:

  • express-zod-api/tests/system.spec.ts
📚 Learning: 2025-08-01T09:48:13.742Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#0
File: :0-0
Timestamp: 2025-08-01T09:48:13.742Z
Learning: In express-zod-api, when migrating from Zod v3 to v4, the correct approach for internal type imports is to change from `import type { $ZodType } from "zod/v4/core"` to `import { z } from "zod"` and then use `z.core.$ZodType`. The zod/v4/core module is reexported as z.core by the main zod package, making this a valid and working approach.

Applied to files:

  • zod-plugin/index.ts
📚 Learning: 2025-05-28T18:58:10.064Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2428
File: express-zod-api/src/index.ts:44-44
Timestamp: 2025-05-28T18:58:10.064Z
Learning: The type-only import `import type {} from "qs";` in express-zod-api/src/index.ts is necessary to avoid TS2742 errors for exported functions like attachRouting, makeRequestMock, testEndpoint, and testMiddleware that have types depending on types/qs. This import provides the reference TypeScript needs to infer portable type names.

Applied to files:

  • zod-plugin/index.ts
  • express-zod-api/tsdown.config.ts
📚 Learning: 2025-06-02T21:11:20.768Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2697
File: CHANGELOG.md:5-5
Timestamp: 2025-06-02T21:11:20.768Z
Learning: In the express-zod-api repository, RobinTail follows a release workflow where package.json version is only updated on the master branch after merging all planned release changes. Changelog entries may show future version numbers while package.json remains at the previous version during feature development, and this is intentional workflow, not a version inconsistency that needs to be flagged.

Applied to files:

  • express-zod-api/package.json
📚 Learning: 2025-05-27T20:27:17.015Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/src/json-schema-helpers.ts:1-3
Timestamp: 2025-05-27T20:27:17.015Z
Learning: Ramda is correctly listed as a dependency in express-zod-api/package.json, so imports of ramda utilities are properly supported.

Applied to files:

  • express-zod-api/package.json
📚 Learning: 2025-08-08T11:59:04.814Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2878
File: zod-plugin/runtime.ts:39-42
Timestamp: 2025-08-08T11:59:04.814Z
Learning: express-zod-api/zod-plugin/runtime.ts (TypeScript): Do not memoize the ZodType.prototype.brand getter. The current design returning setBrand.bind(this) on each access is intentional/preferred; avoid redefining "brand" as a data property per instance.

Applied to files:

  • express-zod-api/src/common-helpers.ts
🔇 Additional comments (13)
package.json (1)

27-27: tsdown migration verified across all workspaces

All checks passed—no lingering tsup references and tsdown is configured in each workspace.

Findings:

  • No TSUP_* environment variables found
  • No tsup.config.* files present
  • No tsup invocations in any package.json scripts
  • tsdown.config.ts exists in:
    • express-zod-api/tsdown.config.ts
    • migration/tsdown.config.ts
    • zod-plugin/tsdown.config.ts

Proceed with approving these changes.

express-zod-api/tests/result-helpers.spec.ts (1)

119-121: LGTM: env stub migrated to TSDOWN_STATIC

The rename from TSUP_STATIC to TSDOWN_STATIC, alongside NODE_ENV, aligns with the build tool migration and keeps the production-mode behavior consistent in tests.

express-zod-api/tests/system.spec.ts (1)

21-22: LGTM: production-mode stub updated to TSDOWN_STATIC

Stubbing TSDOWN_STATIC to "production" here matches the new env strategy. The later vi.unstubAllEnvs() in afterAll ensures isolation between suites.

express-zod-api/tests/last-resort.spec.ts (1)

13-15: LGTM: TSDOWN_STATIC stub applied in Last Resort tests

The env variable rename is consistently applied and paired with NODE_ENV. Cleanup with vi.unstubAllEnvs() is present.

zod-plugin/package.json (1)

21-21: tsdown configuration validated – ready to approve

  • Confirmed zod-plugin/tsdown.config.ts exists.
  • Verifed it includes
    attw: { profile: "esmOnly", level: "error" },
    matching the previous setup.
  • No leftover tsup references detected.
express-zod-api/src/server.ts (1)

35-35: Migration Verified – No leftover TSUP_ references*
Switch to TSDOWN_BUILD is consistent; default fallback unchanged. Confirmed via rg that no TSUP_* env vars remain.

express-zod-api/tests/builtin-logger.spec.ts (1)

46-46: LGTM: Test env stub updated to TSDOWN_STATIC

Matches the new isProduction() source of truth. The afterEach vi.unstubAllEnvs() above keeps tests isolated.

migration/package.json (1)

20-20: ✅ Build Script Migration Verified

The build: "tsdown" script is correctly wired:

  • tsdown.config.ts exists in all workspaces:
    • migration/tsdown.config.ts
    • express-zod-api/tsdown.config.ts
    • zod-plugin/tsdown.config.ts
  • Each config includes the required attw integration:
    • migration/tsdown.config.ts: attw: { profile: "esmOnly", level: "error" }
    • express-zod-api/tsdown.config.ts: attw: { profile: "esmOnly", level: "error" }
    • zod-plugin/tsdown.config.ts: attw: { profile: "esmOnly", level: "error" }

No further changes needed—ready to ship!

express-zod-api/package.json (1)

19-19: LGTM: build script updated to tsdown and no lingering semver imports

Verified that all semver imports/requires have been removed. Approving these changes.

express-zod-api/src/common-helpers.ts (1)

142-144: Env switch to TSDOWN_STATIC is correct and consistent with the new build config.

The memoization key now aligns with express-zod-api/tsdown.config.ts where TSDOWN_STATIC is defined. No behavior change beyond the env rename.

zod-plugin/tsdown.config.ts (1)

3-7: Approved: tsdown config for zod-plugin is correct
The entry file at zod-plugin/index.ts exists, and the minimal tsdown config (entry/minify/attw) aligns with our migration goals.

migration/tsdown.config.ts (1)

3-8: Approve migration package config
Verified migration/index.ts exists. skipNodeModulesBundle: true, attw profile (esmOnly, level error), and minify: true align with the other packages.

express-zod-api/tsdown.config.ts (1)

11-13: Define mappings look correct for tsdown/esbuild.

Quoting the values ensures string literal substitution; keys match the new consumers (isProduction and debug logging).

@RobinTail RobinTail added the coverage Additional tests label Aug 15, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
zod-plugin/tsdown.config.ts (2)

8-10: Confirm whether banner.dts must live under dts.banner in tsdown.

Depending on tsdown’s config schema, the DTS banner may need to be nested under the dts option (e.g., dts.banner) rather than banner.dts. If your local type-checking or build warns, switch to the nested form.

Apply this diff if required:

-    banner: {
-      dts: "import './augmentation.js';",
-    },
+    dts: {
+      banner: "import './augmentation.js';",
+    },

13-15: Rename the augmentation file to .d.ts for reliable retention

To ensure tsdown always preserves your module augmentations, rename and update your entry:

  • Rename on disk:

    mv zod-plugin/augmentation.ts zod-plugin/augmentation.d.ts
    
  • In zod-plugin/tsdown.config.ts (around lines 13–15), update the entry list:

    -  entry: ["augmentation.ts"],
    +  entry: ["augmentation.d.ts"],

This makes the source a true declaration file, aligning with tsdown’s migration notes and hardening augmentation retention.
[optional_refactors_recommended]

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 305ff1e and 202e0bb.

📒 Files selected for processing (2)
  • zod-plugin/index.ts (0 hunks)
  • zod-plugin/tsdown.config.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • zod-plugin/index.ts
🔇 Additional comments (1)
zod-plugin/tsdown.config.ts (1)

3-16: Config structure and two-entry strategy look solid.

Good separation of the runtime bundle and the DTS-only augmentation; the NodeNext-friendly import in the DTS banner is a clean way to include augmentations without a runtime side effect. attw gating is also a nice touch.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (4)
compat-test/dts.spec.ts (4)

6-11: Avoid hard-coded node_modules paths; resolve package locations programmatically

Hard-coding "./node_modules/..." is brittle across package managers (pnpm/yarn/npm), hoisting, and workspace setups. Resolve the installed package paths using createRequire + require.resolve, then read the files from those resolved roots. Also, prefer a regex for the import assertion to allow either single or double quotes and optional semicolons.

Apply this diff:

-    const fwDts = await readFile(
-      "./node_modules/express-zod-api/dist/index.d.ts",
-      "utf-8",
-    );
-    expect(fwDts).toMatch(`import "@express-zod-api/zod-plugin";`);
+    const fwDts = await readFile(fwDtsPath, "utf-8");
+    expect(fwDts).toMatch(/import\s+['"]@express-zod-api\/zod-plugin['"];?/);

14-19: Use resolver-based path and robust match ('.' is special in regex)

The current test path assumes nested node_modules and may break with hoisting. Also, toMatch(string) treats the string as a regex; the '.' in './augmentation.js' is a wildcard. Use the resolved path and an escaped regex that allows both quote styles.

-    const pluginDts = await readFile(
-      "./node_modules/express-zod-api/node_modules/@express-zod-api/zod-plugin/dist/index.d.ts",
-      "utf-8",
-    );
-    expect(pluginDts).toMatch(`import './augmentation.js';`);
+    const pluginDts = await readFile(pluginDtsPath, "utf-8");
+    expect(pluginDts).toMatch(/import\s+['"]\.\/augmentation\.js['"];?/);

22-27: Resolve augmentation.d.ts via the package root and relax quoting

Same resilience/regex concerns as above. Use the resolved file path and allow either quote style.

-    const augDts = await readFile(
-      "./node_modules/express-zod-api/node_modules/@express-zod-api/zod-plugin/dist/augmentation.d.ts",
-      "utf-8",
-    );
-    expect(augDts).toMatch(`declare module "zod"`);
+    const augDts = await readFile(pluginAugDtsPath, "utf-8");
+    expect(augDts).toMatch(/declare module ["']zod["']/);

1-3: Introduce resolver utilities once to keep tests portable across package managers

Add createRequire + path helpers and precompute the DTS file paths so all tests are independent of node_modules layout.

 import { describe, test, expect } from "vitest";
 import { readFile } from "node:fs/promises";
 
+import { createRequire } from "node:module";
+import { dirname, join } from "node:path";
+
+const require = createRequire(import.meta.url);
+const ezapiPkgJson = require.resolve("express-zod-api/package.json");
+const ezapiRoot = dirname(ezapiPkgJson);
+const fwDtsPath = join(ezapiRoot, "dist/index.d.ts");
+// Resolve the plugin as if required from express-zod-api to avoid mismatches under hoisting
+const pluginPkgJson = require.resolve("@express-zod-api/zod-plugin/package.json", { paths: [ezapiRoot] });
+const pluginRoot = dirname(pluginPkgJson);
+const pluginDtsPath = join(pluginRoot, "dist/index.d.ts");
+const pluginAugDtsPath = join(pluginRoot, "dist/augmentation.d.ts");
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 202e0bb and a167aa8.

📒 Files selected for processing (4)
  • compat-test/dts.spec.ts (1 hunks)
  • express-zod-api/tsdown.config.ts (1 hunks)
  • migration/tsdown.config.ts (1 hunks)
  • zod-plugin/tsdown.config.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • zod-plugin/tsdown.config.ts
  • migration/tsdown.config.ts
  • express-zod-api/tsdown.config.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2428
File: express-zod-api/src/index.ts:44-44
Timestamp: 2025-05-28T18:58:10.064Z
Learning: The type-only import `import type {} from "qs";` in express-zod-api/src/index.ts is necessary to avoid TS2742 errors for exported functions like attachRouting, makeRequestMock, testEndpoint, and testMiddleware that have types depending on types/qs. This import provides the reference TypeScript needs to infer portable type names.
📚 Learning: 2025-05-28T18:58:10.064Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2428
File: express-zod-api/src/index.ts:44-44
Timestamp: 2025-05-28T18:58:10.064Z
Learning: The type-only import `import type {} from "qs";` in express-zod-api/src/index.ts is necessary to avoid TS2742 errors for exported functions like attachRouting, makeRequestMock, testEndpoint, and testMiddleware that have types depending on types/qs. This import provides the reference TypeScript needs to infer portable type names.

Applied to files:

  • compat-test/dts.spec.ts
📚 Learning: 2025-05-27T19:30:51.885Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: compat-test/sample.ts:1-1
Timestamp: 2025-05-27T19:30:51.885Z
Learning: Files in compat-test/ directories, especially those named sample.ts or similar, are often test fixtures for migration scripts and may intentionally contain deprecated or "incorrect" code that the migration tooling is designed to fix. These should not be flagged as issues.

Applied to files:

  • compat-test/dts.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build (24.0.0)
🔇 Additional comments (1)
compat-test/dts.spec.ts (1)

4-28: Solid DTS wiring checks — nice coverage of the migration behavior

These tests capture the critical DTS relationships (framework -> plugin -> augmentation) that tsdown can sometimes drop. This will guard against regressions in future tool updates.

@RobinTail
Copy link
Owner Author

✅ QA passed

🏁 Ready

@RobinTail RobinTail merged commit d8f8836 into master Aug 16, 2025
5 checks passed
@RobinTail RobinTail deleted the use-tsdown branch August 16, 2025 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI/CD coverage Additional tests dependencies Pull requests that update a dependency file external bug it's a bug, but in a dependency patched Some dependency is patched refactoring The better way to achieve the same result

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants