Skip to content

Conversation

@edison1105
Copy link
Member

@edison1105 edison1105 commented Nov 6, 2025

Reverts #13397
Close #14060

Summary by CodeRabbit

  • Tests

    • Removed test case for expression type casting
  • Chores

    • Refactored internal expression transformation logic to streamline identifier text processing while maintaining existing functionality

@coderabbitai
Copy link

coderabbitai bot commented Nov 6, 2025

Walkthrough

The PR simplifies Vue compiler expression transformation logic by removing TypeScript node-type conditional gating and adjusting leading/trailing text handling around identifiers, along with removing a corresponding test case.

Changes

Cohort / File(s) Summary
Test removal
packages/compiler-core/__tests__/transforms/transformExpressions.spec.ts
Removes single "expression with type" test case that verified casting expressions to types and resulting snapshot behavior.
Transform logic simplification
packages/compiler-core/src/transforms/transformExpression.ts
Removes TS_NODE_TYPES import and conditional gating; refactors processExpression to always prefix identifiers and handle trailing text without TypeScript node-type checks, eliminating special-case behavior for first identifier and conditional trailing text preservation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Logic behavior change: The removal of TS_NODE_TYPES conditionals alters how leading/trailing text is emitted in compound expressions; requires validation that the new always-prefix approach produces correct output across existing test cases.
  • Interdependency with removed test: Ensure the deleted test case doesn't represent important edge-case coverage that should be preserved or migrated elsewhere.

Possibly related PRs

Suggested labels

scope: compiler, ready for review

Poem

🐰 No more TypeScript node gates to mind,
Identifiers prefixed, cleanly designed,
Trailing text flows where it should go,
Simpler logic, a lighter show!

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 accurately describes the main change: reverting a previous commit that removed types for expressions in compiler-core, which aligns with the actual changes shown in the raw summary.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ 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 revert-13397-fix-expression-core

📜 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 5cf0097 and 3899301.

⛔ Files ignored due to path filters (1)
  • packages/compiler-core/__tests__/transforms/__snapshots__/transformExpressions.spec.ts.snap is excluded by !**/*.snap
📒 Files selected for processing (2)
  • packages/compiler-core/__tests__/transforms/transformExpressions.spec.ts (0 hunks)
  • packages/compiler-core/src/transforms/transformExpression.ts (2 hunks)
💤 Files with no reviewable changes (1)
  • packages/compiler-core/tests/transforms/transformExpressions.spec.ts
🔇 Additional comments (2)
packages/compiler-core/src/transforms/transformExpression.ts (2)

375-377: Verify trailing text handling for TypeScript expressions.

The trailing text assembly logic is correct and straightforward. However, the enriched summary indicates that a "TS-node conditional gate" was removed from this section. Please verify that TypeScript expressions with trailing type syntax (e.g., type assertions, non-null assertions like foo!, or type annotations) are handled correctly.

Consider adding test coverage for expressions with TypeScript-specific trailing syntax if not already present.


356-359: ****

The review comment's concerns do not apply to the code being reviewed. TS_NODE_TYPES is not imported or used in transformExpression.ts, and the leading text assembly logic at lines 356-359 contains no TS-specific conditionals. The enriched summary's claim that "TS node types involved" conditionals were removed from this file is inaccurate—there were no such conditionals in the code under review. The code correctly handles compound expression assembly for all expression types without special TypeScript casing.

Likely an incorrect or invalid review comment.


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

@github-actions
Copy link

github-actions bot commented Nov 6, 2025

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 103 kB 38.9 kB 35 kB
vue.global.prod.js 161 kB 58.8 kB 52.3 kB

Usages

Name Size Gzip Brotli
createApp (CAPI only) 46.9 kB 18.3 kB 16.8 kB
createApp 55 kB 21.4 kB 19.6 kB
createSSRApp 59.3 kB 23.1 kB 21.1 kB
defineCustomElement 60.6 kB 23.1 kB 21.1 kB
overall 69.3 kB 26.6 kB 24.3 kB

@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 6, 2025

Open in StackBlitz

@vue/compiler-core

npm i https://pkg.pr.new/@vue/compiler-core@14062

@vue/compiler-dom

npm i https://pkg.pr.new/@vue/compiler-dom@14062

@vue/compiler-sfc

npm i https://pkg.pr.new/@vue/compiler-sfc@14062

@vue/compiler-ssr

npm i https://pkg.pr.new/@vue/compiler-ssr@14062

@vue/reactivity

npm i https://pkg.pr.new/@vue/reactivity@14062

@vue/runtime-core

npm i https://pkg.pr.new/@vue/runtime-core@14062

@vue/runtime-dom

npm i https://pkg.pr.new/@vue/runtime-dom@14062

@vue/server-renderer

npm i https://pkg.pr.new/@vue/server-renderer@14062

@vue/shared

npm i https://pkg.pr.new/@vue/shared@14062

vue

npm i https://pkg.pr.new/vue@14062

@vue/compat

npm i https://pkg.pr.new/@vue/compat@14062

commit: 3899301

@edison1105 edison1105 merged commit 11ec51a into main Nov 7, 2025
16 checks passed
@edison1105 edison1105 deleted the revert-13397-fix-expression-core branch November 7, 2025 00:52
@volarname
Copy link

please any ETA for vue hotfix version?

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.

Regression in 3.5.23: fails to parse ! in typescript

3 participants