Skip to content

Cleanup extensions#3088

Merged
RobinTail merged 2 commits intomasterfrom
cleanup-extensions
Nov 24, 2025
Merged

Cleanup extensions#3088
RobinTail merged 2 commits intomasterfrom
cleanup-extensions

Conversation

@RobinTail
Copy link
Copy Markdown
Owner

@RobinTail RobinTail commented Nov 24, 2025

Summary by CodeRabbit

  • Chores
    • Removed an explicit TypeScript compiler option that allowed importing TypeScript file extensions; no other compiler behavior changed.
  • Tests
    • Simplified test imports by loading dependencies at module initialization rather than at runtime, improving test clarity and consistency.

✏️ Tip: You can customize this high-level summary in your review settings.

@RobinTail RobinTail added the refactoring The better way to achieve the same result label Nov 24, 2025
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Nov 24, 2025

Walkthrough

Removed allowImportingTsExtensions from tsconfig.json and converted a runtime dynamic import in cjs-test/quick-start.spec.ts to a static top-level import; behavior and exported APIs unchanged.

Changes

Cohort / File(s) Change Summary
TypeScript configuration
tsconfig.json
Removed allowImportingTsExtensions: true compiler option.
Test import adjustment
cjs-test/quick-start.spec.ts
Replaced a runtime dynamic import of givePort with a static top-level import from ../tools/ports; usage of givePort("cjs") unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Potential review focus:

  • cjs-test/quick-start.spec.ts: confirm test loading order and any side-effects from moving import to module scope.
  • tsconfig.json: verify build/IDE behavior in environments that previously relied on importing TS extensions.

Possibly related PRs

  • Root tsconfig #2890 — Modifies TypeScript project configuration and relates to tsconfig.json changes.

Poem

🐰 I hopped through files, small and neat,
Pulled a line and fixed my feet.
Imports rise at module start,
Config trimmed — a tiny art.
Sniff, nibble, sprint, and then repeat. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Cleanup extensions' directly relates to the main change: removing the allowImportingTsExtensions option from tsconfig.json, which aligns with the PR's stated objective to remove the allowance for TypeScript extensions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch cleanup-extensions

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ce4b741 and cec8fbe.

📒 Files selected for processing (1)
  • cjs-test/quick-start.spec.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: RobinTail
Repo: RobinTail/express-zod-api PR: 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.
Learnt from: RobinTail
Repo: RobinTail/express-zod-api PR: 2974
File: express-zod-api/src/zts-helpers.ts:2-3
Timestamp: 2025-09-29T06:00:10.830Z
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.
Learnt from: RobinTail
Repo: RobinTail/express-zod-api PR: 0
File: :0-0
Timestamp: 2025-09-29T03:35:55.561Z
Learning: In the express-zod-api repository, packages are built using tsdown before publishing. Source code with .ts extensions is not published - only the built JavaScript bundles and declaration files in the dist/ directory are published to npm. This means .ts extensions in source imports don't affect consumers.
Learnt from: RobinTail
Repo: RobinTail/express-zod-api PR: 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.
Learnt from: RobinTail
Repo: RobinTail/express-zod-api PR: 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.
📚 Learning: 2025-05-28T18:58:10.064Z
Learnt from: RobinTail
Repo: RobinTail/express-zod-api PR: 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:

  • cjs-test/quick-start.spec.ts
📚 Learning: 2025-06-02T21:08:56.475Z
Learnt from: RobinTail
Repo: RobinTail/express-zod-api PR: 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:

  • cjs-test/quick-start.spec.ts
📚 Learning: 2025-10-02T17:42:48.840Z
Learnt from: RobinTail
Repo: RobinTail/express-zod-api PR: 0
File: :0-0
Timestamp: 2025-10-02T17:42:48.840Z
Learning: In express-zod-api v25 (ESM-only), the `.example()` method error occurs when user code runs as CommonJS. Express Zod API patches Zod's ESM bundle with `.example()`, but CommonJS code requires a separate CJS bundle instance that lacks this patch. Users must run their code as ESM by: (1) setting `"type": "module"` in package.json, (2) using `.mts` or `.mjs` file extensions, or (3) using tools like `tsx` or `vite-node` that provide their own ESM-compatible compilation.

Applied to files:

  • cjs-test/quick-start.spec.ts
🔇 Additional comments (1)
cjs-test/quick-start.spec.ts (1)

2-13: Static import of givePort aligns with tsconfig cleanup

Switching from a dynamic import() to a static top‑level import { givePort } from "../tools/ports"; keeps behavior the same while avoiding .ts/.js extension concerns and fits the removal of allowImportingTsExtensions in tsconfig. No issues spotted with how port is computed and used in the test. Based on learnings, this is consistent with the repo’s tooling and bundling approach.


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

Comment @coderabbitai help to get the list of available commands and usage tips.

@RobinTail RobinTail merged commit df3ebb4 into master Nov 24, 2025
13 checks passed
@RobinTail RobinTail deleted the cleanup-extensions branch November 24, 2025 16:24
@coveralls-official
Copy link
Copy Markdown

Coverage Status

coverage: 100.0%. remained the same
when pulling cec8fbe on cleanup-extensions
into 7a553e4 on master.

@coderabbitai coderabbitai Bot mentioned this pull request Nov 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactoring The better way to achieve the same result

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant