Skip to content

Conversation

@libondev
Copy link

@libondev libondev commented Jun 10, 2025

reimpl: #617

This pr is similar to the implementation of vuejs/pinia#2954, which transfers the work of automatically injecting hmr code to the plugin part, and adds an option to control whether the plugin is enabled or not.

Summary by CodeRabbit

  • New Features

    • Added configurable automatic Hot Module Replacement (HMR) handling for Vue Router, enabled by default and overridable via a new public option and default settings.
    • Automatically injects the runtime hook import and invokes it when a router instance is detected, respecting include/exclude filters.
  • Tests

    • Added a comprehensive test suite covering detection, injection, filtering, and edge cases to ensure safe, non-duplicating transformations.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jun 10, 2025

Walkthrough

Adds an unplugin-based auto-HMR plugin for Vue Router that detects router declarations, injects a handleHotUpdate import and invocation when needed, and exposes configuration via new AutoHmrOptions and DEFAULT_AUTO_HMR_OPTIONS. Integrates the plugin when options.autoHmr is enabled.

Changes

Cohort / File(s) Change Summary
Auto HMR plugin implementation
src/auto-hmr/index.ts
New exported createAutoHmrPlugin, AutoHmrOptions, and DEFAULT_AUTO_HMR_OPTIONS. Plugin parses AST to find createRouter usage, conditionally prepends an import for handleHotUpdate from a configurable modulePath, and appends a handleHotUpdate(router) call. Uses include/exclude filter and runs in post phase.
AST helpers
src/auto-hmr/ast.ts
New exported AST utility functions: nameFromDeclaration, getRouterDeclaration, getHandleHotUpdateDeclaration, and hasHandleHotUpdateCall for detecting router declarations, imports, and calls.
Tests for auto-HMR
src/auto-hmr/auto-hmr.spec.ts
New comprehensive unit tests covering file filtering, router detection (createRouter / experimental_createRouter), import injection, invocation injection, custom modulePath, and various edge cases.
Integration & options
src/index.ts, src/options.ts
Import and conditionally append the auto-HMR plugin when options.autoHmr is enabled; add autoHmr?: AutoHmrOptions to Options and set DEFAULT_OPTIONS.autoHmr = DEFAULT_AUTO_HMR_OPTIONS.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Inspect AST helper correctness and traversal short-circuiting (src/auto-hmr/ast.ts).
  • Verify router detection and variable-name extraction logic in src/auto-hmr/index.ts.
  • Confirm safe insertion points for import and call injection and handling of existing imports/calls.
  • Review tests for coverage and realistic AST mocks in src/auto-hmr/auto-hmr.spec.ts.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main changes: adding a new auto-hmr plugin and introducing an autoHmr option to the codebase.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

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

♻️ Duplicate comments (1)
src/auto-hmr/index.ts (1)

77-77: Regex reliability concern.

Same issue as line 35 - regex matching in code can be unreliable.

🧹 Nitpick comments (1)
src/auto-hmr/index.ts (1)

35-35: Consider regex reliability.

The regex /handleHotUpdate\([\s\S]*?\)/ may match unintended occurrences like comments or strings. Consider using AST-based detection instead.

-  const handleHotUpdateCallRegex = /handleHotUpdate\([\s\S]*?\)/

Use AST parsing to detect function calls more reliably:

function hasHandleHotUpdateCall(ast: any): boolean {
  // Walk AST to find CallExpression with callee.name === 'handleHotUpdate'
}
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between e62ef6f and aee9b7a.

📒 Files selected for processing (3)
  • src/auto-hmr/index.ts (1 hunks)
  • src/index.ts (2 hunks)
  • src/options.ts (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/index.ts (2)
src/auto-hmr/index.ts (1)
  • createAutoHmrPlugin (34-101)
src/core/moduleConstants.ts (1)
  • MODULE_ROUTES_PATH (6-6)
🔇 Additional comments (11)
src/options.ts (1)

216-220: LGTM!

Clean addition of the new option with proper documentation.

src/index.ts (2)

24-24: Clean import addition.


206-212: Good integration pattern.

Follows the same conditional plugin inclusion pattern as the experimental autoExportsDataLoaders.

src/auto-hmr/index.ts (8)

4-6: Helper function is correct.


8-15: Router detection logic is sound.


17-28: Import detection logic is correct.


42-42: Virtual module filtering is correct.


46-48: Early return optimization is good.


82-86: Import injection looks correct.


89-91: Function call injection is correct.


93-95: Code transformation approach is sound.

libondev and others added 3 commits June 10, 2025 14:30
@libondev
Copy link
Author

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jun 10, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@libondev
Copy link
Author

@posva PTAL

Copy link
Owner

@posva posva left a comment

Choose a reason for hiding this comment

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

Thanks and sorry for the delay!

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

♻️ Duplicate comments (1)
src/auto-hmr/index.ts (1)

69-89: Fix the TypeScript type error properly.

The @ts-expect-error suppresses a type mismatch. The AST body should be typed correctly rather than using a type suppression comment.

Apply this diff to fix:

-      // @ts-expect-error
-      for (const node of ast.body) {
+      for (const node of ast.body as any[]) {

Alternatively, import and use the proper estree types for Program to type ast.body correctly.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c77f8c4 and c01f690.

📒 Files selected for processing (5)
  • src/auto-hmr/ast.ts (1 hunks)
  • src/auto-hmr/auto-hmr.spec.ts (1 hunks)
  • src/auto-hmr/index.ts (1 hunks)
  • src/index.ts (2 hunks)
  • src/options.ts (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/index.ts
  • src/options.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-16T13:01:42.709Z
Learnt from: posva
Repo: posva/unplugin-vue-router PR: 700
File: src/codegen/generateParamParsers.ts:91-94
Timestamp: 2025-08-16T13:01:42.709Z
Learning: In the unplugin-vue-router codebase, path normalization for import specifiers is handled elsewhere in the system (not at individual call sites like generateParamParsers.ts). Individual functions should not normalize paths themselves as it violates separation of concerns.

Applied to files:

  • src/auto-hmr/auto-hmr.spec.ts
  • src/auto-hmr/index.ts
📚 Learning: 2025-08-15T16:11:02.627Z
Learnt from: posva
Repo: posva/unplugin-vue-router PR: 700
File: src/codegen/generateRouteResolver.ts:0-0
Timestamp: 2025-08-15T16:11:02.627Z
Learning: In src/codegen/generateRouteResolver.ts, the user wants comment alignment preserved in the generated resolver code, even when fixing potential runtime errors with String.repeat().

Applied to files:

  • src/auto-hmr/index.ts
📚 Learning: 2025-08-23T12:49:31.747Z
Learnt from: posva
Repo: posva/unplugin-vue-router PR: 700
File: src/core/tree.ts:313-331
Timestamp: 2025-08-23T12:49:31.747Z
Learning: The regexp getter in src/core/tree.ts correctly processes the root node to add the leading slash. The empty pathSegment from root becomes an empty string when escaped, but the concatenation logic `"" + (re ? '\\/' : '') + re` properly adds the leading slash that routes require.

Applied to files:

  • src/auto-hmr/index.ts
🧬 Code graph analysis (2)
src/auto-hmr/auto-hmr.spec.ts (1)
src/auto-hmr/index.ts (3)
  • createAutoHmrPlugin (39-116)
  • transform (54-114)
  • DEFAULT_AUTO_HMR_OPTIONS (30-37)
src/auto-hmr/index.ts (1)
src/auto-hmr/ast.ts (4)
  • getRouterDeclaration (8-16)
  • nameFromDeclaration (4-6)
  • getHandleHotUpdateDeclaration (18-29)
  • hasHandleHotUpdateCall (31-72)
🔇 Additional comments (9)
src/auto-hmr/auto-hmr.spec.ts (1)

1-754: LGTM! Comprehensive test coverage.

The test suite thoroughly covers file filtering, router detection (both createRouter and experimental_createRouter), declaration detection, import/call injection logic, and various edge cases. The mock setup is appropriate and the test organization is clear.

src/auto-hmr/ast.ts (3)

4-6: LGTM!

Clean null-safe extraction of identifier name from declaration.


8-16: LGTM!

Correctly identifies router declarations from createRouter or experimental_createRouter calls.


18-29: LGTM!

Properly checks for handleHotUpdate import from the specified module path.

src/auto-hmr/index.ts (5)

11-28: LGTM!

Clear interface definition with proper documentation for the auto-HMR options.


30-37: LGTM!

Sensible defaults for auto-HMR: targets common router file patterns and uses the standard module path.


43-49: LGTM!

Regex pre-filter and file filter setup are efficient and correctly configured.


54-61: LGTM!

Proper early returns for virtual modules, non-matching files, and files without createRouter calls optimize performance.


91-111: LGTM!

The injection logic correctly adds import and call only when needed, avoiding duplicates. Code generation using string concatenation is straightforward.

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 (1)
src/auto-hmr/ast.ts (1)

56-56: Remove redundant check.

Line 35 already verifies typeof node !== 'object'.

-    if (typeof node !== 'object') return false
-
     for (const key in node) {
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c01f690 and aecc136.

📒 Files selected for processing (1)
  • src/auto-hmr/ast.ts (1 hunks)
🔇 Additional comments (4)
src/auto-hmr/ast.ts (4)

4-6: LGTM.

Clean null-safe implementation.


8-16: LGTM.

The AND chain ensures x.init exists before accessing x.init.callee on line 12.


18-29: LGTM.

The guard on line 20 ensures safe property access on subsequent lines.


31-75: Past review concern addressed.

The WeakSet on line 32 now guards against circular references as requested.

@libondev
Copy link
Author

I made some adjustments to the code. Now the autoHmr function is enabled by default. Users can also configure which files need to be scanned and where to import handleHotUpdate.

@libondev libondev requested a review from posva November 21, 2025 07:06
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.

2 participants