Skip to content

Conversation

@ahnpnl
Copy link
Collaborator

@ahnpnl ahnpnl commented Dec 1, 2025

Summary

Fixes #5130

Test plan

Green CI

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

N.A.

Summary by CodeRabbit

  • New Features

    • Added support for Node 20 module kind resolution
  • Improvements

    • Refactored diagnostic handling for more immediate feedback during module resolution
  • Tests

    • Introduced end-to-end testing infrastructure for module compatibility scenarios
    • Expanded test coverage for module resolution edge cases

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

@coderabbitai
Copy link

coderabbitai bot commented Dec 1, 2025

Walkthrough

Adds E2E test infrastructure for a hybrid-module feature, including test specifications and Jest configurations for both CommonJS and ES modules. Refactors compiler diagnostic handling to use explicit diagnostic raising via raiseDiagnostics() instead of direct array mutation. Extends modern Node module kind recognition to include Node20 alongside existing Node16 and NodeNext.

Changes

Cohort / File(s) Summary
E2E Hybrid-Module Test Infrastructure
e2e/hybrid-module/__tests__/hybrid-module.spec.ts, e2e/hybrid-module/jest-compiler-cjs.config.ts, e2e/hybrid-module/jest-compiler-esm.config.ts, e2e/hybrid-module/package.json, e2e/hybrid-module/tsconfig.spec.json
Introduces new end-to-end test suite for hybrid-module feature with Jest configurations for CommonJS (ts-jest, tsconfig at <rootDir>/tsconfig.spec.json) and ES modules (ts-jest with useESM: true). Both configs ignore diagnostic code 151002. Includes minimal package.json and test scaffold.
Compiler Diagnostic Handling Refactor
src/legacy/compiler/ts-compiler.ts, src/legacy/compiler/ts-compiler.spec.ts
Changes modern Node module diagnostics to be raised immediately via this.configSet.raiseDiagnostics() instead of appended to diagnostics array. Updates corresponding test to verify log warnings instead of diagnostics array entries for modern Node resolution case.
Module Kind Recognition
src/transpilers/typescript/transpile-module.ts
Expands isModernNodeModuleKind function to recognize Node20 (ModuleKind.Node20 / literal 102) as a modern Node module kind in addition to Node16 and NodeNext.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

  • Focus areas:
    • Verify raiseDiagnostics() call in ts-compiler.ts correctly replaces the previous diagnostic array mutation and maintains intended behavior
    • Confirm test changes in ts-compiler.spec.ts properly validate diagnostic raising through log assertions
    • Validate Node20 constant inclusion in module kind check is complete and doesn't break existing logic
    • Ensure diagnostic code 151002 is properly ignored in new E2E Jest configurations

Possibly related PRs

Suggested reviewers

  • kulshekhar

Poem

🐰 Diagnostics raised with purpose true,
Node20 joins the module crew,
Hybrid tests now stand so tall,
One-fifty-two we ignore it all!

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: log hybrid module as warning instead of failing tests' clearly and concisely summarizes the main change—converting a diagnostic from failure to warning for hybrid module handling.
Linked Issues check ✅ Passed The PR addresses issue #5130 by modifying how the TS151002 hybrid module diagnostic is handled, changing it from a test-failing error to a warning log, which meets the linked issue requirements.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the hybrid module diagnostic handling: updates to compiler logic, test adjustments, config files, and e2e test scaffolding are all in scope.
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 fix/diagnostics-hybrid

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.

@ahnpnl ahnpnl force-pushed the fix/diagnostics-hybrid branch from 6193853 to b9cbdd1 Compare December 1, 2025 14:38
@ahnpnl ahnpnl force-pushed the fix/diagnostics-hybrid branch 2 times, most recently from 86805a1 to be2a95d Compare December 1, 2025 14:57
@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 1, 2025

@ahnpnl ahnpnl marked this pull request as ready for review December 1, 2025 15:09
@ahnpnl ahnpnl requested a review from kulshekhar as a code owner December 1, 2025 15:09
Copy link

@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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 141e5af and be2a95d.

📒 Files selected for processing (8)
  • e2e/hybrid-module/__tests__/hybrid-module.spec.ts (1 hunks)
  • e2e/hybrid-module/jest-compiler-cjs.config.ts (1 hunks)
  • e2e/hybrid-module/jest-compiler-esm.config.ts (1 hunks)
  • e2e/hybrid-module/package.json (1 hunks)
  • e2e/hybrid-module/tsconfig.spec.json (1 hunks)
  • src/legacy/compiler/ts-compiler.spec.ts (1 hunks)
  • src/legacy/compiler/ts-compiler.ts (1 hunks)
  • src/transpilers/typescript/transpile-module.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ahnpnl
Repo: kulshekhar/ts-jest PR: 4976
File: examples/js-with-babel/package.json:14-19
Timestamp: 2025-08-05T08:03:52.583Z
Learning: ts-jest version 29 supports both Jest 29 and Jest 30, so having ts-jest 29.x with Jest 30.x is not a compatibility issue and should not be flagged as a version mismatch problem.
📚 Learning: 2025-08-05T08:03:52.583Z
Learnt from: ahnpnl
Repo: kulshekhar/ts-jest PR: 4976
File: examples/js-with-babel/package.json:14-19
Timestamp: 2025-08-05T08:03:52.583Z
Learning: ts-jest version 29 supports both Jest 29 and Jest 30, so having ts-jest 29.x with Jest 30.x is not a compatibility issue and should not be flagged as a version mismatch problem.

Applied to files:

  • e2e/hybrid-module/jest-compiler-esm.config.ts
  • e2e/hybrid-module/jest-compiler-cjs.config.ts
🧬 Code graph analysis (4)
e2e/hybrid-module/jest-compiler-esm.config.ts (1)
src/constants.ts (1)
  • TS_JS_TRANSFORM_PATTERN (8-8)
src/legacy/compiler/ts-compiler.ts (2)
src/utils/diagnostics.ts (1)
  • TsJestDiagnosticCodes (1-5)
src/utils/messages.ts (1)
  • Helps (30-35)
src/legacy/compiler/ts-compiler.spec.ts (1)
src/utils/messages.ts (1)
  • Helps (30-35)
e2e/hybrid-module/jest-compiler-cjs.config.ts (1)
src/constants.ts (1)
  • TS_JS_TRANSFORM_PATTERN (8-8)
🔇 Additional comments (7)
e2e/hybrid-module/package.json (1)

1-4: LGTM!

The minimal package manifest is appropriate for an e2e test module.

src/legacy/compiler/ts-compiler.ts (1)

213-224: LGTM! Diagnostic handling now respects ignoreCodes configuration.

The shift from array mutation to explicit raiseDiagnostics() ensures the modern Node module diagnostic can be filtered via diagnostics.ignoreCodes, fixing issue #5130.

src/legacy/compiler/ts-compiler.spec.ts (1)

394-398: LGTM! Test correctly validates the new behavior.

The test now verifies that the modern Node module diagnostic is logged as a warning rather than returned in the diagnostics array, confirming the fix allows proper filtering.

e2e/hybrid-module/tsconfig.spec.json (1)

1-3: LGTM!

The minimal tsconfig appropriately extends the base configuration for the e2e test environment.

e2e/hybrid-module/jest-compiler-cjs.config.ts (1)

1-17: LGTM! Configuration correctly demonstrates diagnostic filtering.

The ignoreCodes: [151002] configuration validates that the fix allows the hybrid module diagnostic to be suppressed as intended.

e2e/hybrid-module/jest-compiler-esm.config.ts (1)

1-19: LGTM! ESM configuration correctly demonstrates diagnostic filtering.

The ESM variant properly includes ignoreCodes: [151002] alongside useESM: true, validating that the fix works in ESM mode.

src/transpilers/typescript/transpile-module.ts (1)

51-57: The literal value 102 for ModuleKind.Node20 is correct and aligns with TypeScript 5.9's enum definition (ESNext=99, Node16=100, Node20=102, NodeNext=199).

@ahnpnl ahnpnl merged commit 528d37c into main Dec 1, 2025
34 of 38 checks passed
@ahnpnl ahnpnl deleted the fix/diagnostics-hybrid branch December 1, 2025 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: ts-jest failed to run upgrading from 29.4.4 to 29.4.5

3 participants