feat: automatic HMR code (nuxt only) #2954
Conversation
✅ Deploy Preview for pinia-official canceled.
|
BobbieGoede
changed the title
✅ Deploy Preview for pinia-official canceled.
|
|
Warning Rate limit exceeded@posva has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 12 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
WalkthroughAdds an auto-HMR Vite plugin that injects Pinia HMR handling into store files and removes manual HMR accept blocks from playground stores; updates a playground page to use direct store accessors and a new getter. Changes
Sequence Diagram(s)sequenceDiagram
participant DevServer as Dev Server
participant VitePlugin as Auto-HMR Plugin
participant File as Store File
participant Vite as Vite Transform
Note over VitePlugin,File: Build-time transform
DevServer->>VitePlugin: Scan source files
VitePlugin->>File: Read file (within rootDir)
VitePlugin->>Vite: Parse AST, detect defineStore
Vite->>Vite: Inject import of acceptHMRUpdate\nand HMR accept block (if absent)
Vite->>File: Emit transformed code
Note over DevServer: Runtime HMR
DevServer->>File: Load transformed module
File->>DevServer: import.meta.hot.accept triggers
DevServer->>DevServer: acceptHMRUpdate handles store replacement
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
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. Comment |
commit: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v3 #2954 +/- ##
==========================================
- Coverage 91.28% 91.02% -0.26%
==========================================
Files 18 19 +1
Lines 1618 1672 +54
Branches 231 233 +2
==========================================
+ Hits 1477 1522 +45
- Misses 139 148 +9
Partials 2 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/nuxt/src/auto-hmr-plugin.ts (4)
5-12: Limited defineStore detection may miss valid use cases.The function only matches direct calls to an identifier named
defineStore. It won't detect:
- Member expressions:
pinia.defineStore(...)- Renamed imports:
import { defineStore as createStore } from 'pinia'- Dynamic or computed calls
Consider whether these cases are relevant for the target use case.
30-32: String-based check may cause false positives.The check
code.includes('acceptHMRUpdate')will skip transformation if the string appears anywhere in the file—including comments, string literals, or unrelated code. While this is a reasonable performance optimization, it could prevent legitimate HMR injection in edge cases.Consider whether this trade-off is acceptable for the current scope.
37-66: Plugin only handles the first store per file.The function returns immediately after finding the first
defineStoreusage (line 55), so files with multiple store declarations will only have HMR injected for the first one. Additionally, only top-level declarations are checked—stores defined inside blocks, IIFEs, or conditional exports won't be detected.Ensure this limitation is acceptable for the intended use cases, or track it for future enhancement.
56-63: Import is injected without duplication check.The plugin always prepends
import { acceptHMRUpdate } from 'pinia'without verifying whetheracceptHMRUpdateis already imported. While most bundlers tolerate duplicate imports gracefully, this could lead to linting warnings or edge-case issues.Additionally, consider adding a source map to the returned transformation for better debugging experience.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/nuxt/playground/pages/index.vue(1 hunks)packages/nuxt/playground/stores/counter.ts(1 hunks)packages/nuxt/playground/stores/nested/some-store.ts(0 hunks)packages/nuxt/playground/stores/with-skip-hydrate.ts(0 hunks)packages/nuxt/src/auto-hmr-plugin.ts(1 hunks)packages/nuxt/src/module.ts(2 hunks)
💤 Files with no reviewable changes (2)
- packages/nuxt/playground/stores/with-skip-hydrate.ts
- packages/nuxt/playground/stores/nested/some-store.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/nuxt/src/module.ts (1)
packages/nuxt/src/auto-hmr-plugin.ts (1)
autoRegisterHMRPlugin(18-68)
🔇 Additional comments (3)
packages/nuxt/src/module.ts (1)
82-85: LGTM! Plugin registration is correctly scoped to development.The automatic HMR plugin is properly registered only during development mode, and the necessary dependencies (
nuxtandresolve) are correctly passed.packages/nuxt/playground/pages/index.vue (1)
18-20: LGTM! Template updates align with store changes.The template correctly uses direct property access (
counter.count) and displays the newdoublegetter. The debug output is helpful for verifying state during development.packages/nuxt/playground/stores/counter.ts (1)
19-21: LGTM! Store changes align with automatic HMR approach.The new
doublegetter is correctly implemented, and the removal of manual HMR code (mentioned in the summary) is consistent with the automatic HMR plugin now handling this functionality.
posva
force-pushed
the
feat/nuxt-automatic-hmr-code-append
branch
from
6b0ac78 to
fa0a72d
Compare
| // append HMR code | ||
| return { | ||
| code: [ | ||
| `import { acceptHMRUpdate } from 'pinia'`, |
There was a problem hiding this comment.
I added this so it works with autoImports: false. Let me know I overlooked anything
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/nuxt/playground/pages/index.vue(1 hunks)packages/nuxt/playground/stores/counter.ts(1 hunks)packages/nuxt/playground/stores/nested/some-store.ts(0 hunks)packages/nuxt/playground/stores/with-skip-hydrate.ts(0 hunks)packages/nuxt/src/auto-hmr-plugin.ts(1 hunks)packages/nuxt/src/module.ts(2 hunks)
💤 Files with no reviewable changes (2)
- packages/nuxt/playground/stores/nested/some-store.ts
- packages/nuxt/playground/stores/with-skip-hydrate.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/nuxt/src/module.ts
🔇 Additional comments (2)
packages/nuxt/playground/pages/index.vue (1)
18-21: Getter wiring looks solidBinding to
counter.doublekeeps the template aligned with the new getter while retaining the existing increment flow.packages/nuxt/playground/stores/counter.ts (1)
20-21: Newdoublegetter looks goodStraightforward derived state that pairs cleanly with the updated template.
posva
force-pushed
the
feat/nuxt-automatic-hmr-code-append
branch
from
fa0a72d to
0b05246
Compare
github-project-automation
bot
moved this from 🧑💻 In progress
to ✅ Done
in Pinia Roadmap
2 participants
This is a quick and dirty implementation of a plugin that automatically appends the HMR code to files that register a pinia store (nuxt only), this only works for very basic setups in this state.
This could be implemented using unplugin so that it works in vue projects (without nuxt) as well, in a separate repo if preferred.
Maybe this functionality was already considered, I couldn't find an old discussion or PR suggesting this 🤔
How it works
Given an example project has the following store:
The plugin checks if it contains
defineStoreand does not containacceptHMRUpdate, then walks the top level nodes of the parsed code AST to find the export/variable declaration which uses thedefineStorefunction. This code likely needs to be expanded to cover more use cases.If an export/variable declaration is found, simply append the necessary HMR code:
// my-store.ts export const myStore = defineStore('my-store', /* ... */) +if (import.meta.hot) { + import.meta.hot.accept( + acceptHMRUpdate(myStore, import.meta.hot) + ) +}Summary by CodeRabbit
New Features
Updates