-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix(nuxt): resolve auto-imports in layers #3035
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v3
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for pinia-official canceled.
|
WalkthroughUpdates module to auto-import stores from Nuxt layers by using getLayerDirectories; bumps @nuxt/kit dependency; removes root resolution entry; adds a playground layer with a basic Pinia store and references it in the playground index page. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant Nuxt as Nuxt Core
participant Mod as Module (packages/nuxt/src/module.ts)
participant Kit as @nuxt/kit
participant FS as File System
Dev->>Nuxt: start dev/build
Nuxt->>Mod: initialize module
Mod->>Kit: getLayerDirectories(nuxt)
Kit-->>Mod: [layers...]
alt options.storesDirs present
loop each storesDir
Mod->>FS: addImportsDir(resolve(root.app, storesDir))
loop each layer
Mod->>FS: addImportsDir(resolve(layer.app, storesDir)) Note right of Mod: new: include layer store dirs
end
end
end
FS-->>Nuxt: auto-import metadata (root + layers)
Nuxt-->>Dev: stores auto-imported (e.g., useBasicStore)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Poem
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v3 #3035 +/- ##
==========================================
- Coverage 91.33% 91.07% -0.27%
==========================================
Files 17 17
Lines 1397 1400 +3
Branches 211 212 +1
==========================================
- Hits 1276 1275 -1
- Misses 120 124 +4
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
There was a problem hiding this 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
🧹 Nitpick comments (2)
packages/nuxt/package.json (1)
45-46
: Version bump to @nuxt/kit is fine; align related Nuxt deps to avoid drift.Kit ≥3.19 introduces getLayerDirectories; keeping Nuxt core/schema around 3.19.x avoids subtle type/runtime mismatches in modules and test-utils. (newreleases.io)
Apply:
"devDependencies": { "@nuxt/module-builder": "1.0.1", - "@nuxt/schema": "^3.9.0", - "@nuxt/test-utils": "^3.19.1", - "nuxt": "^3.17.5", + "@nuxt/schema": "^3.19.1", + "@nuxt/test-utils": "^3.19.1", + "nuxt": "^3.19.1", "pinia": "workspace:^", "typescript": "^5.8.3", "vue-tsc": "^2.2.10" },#!/bin/bash # Show Nuxt-related versions and ensure no root-level overrides remain. jq -r '.dependencies["@nuxt/kit"], .devDependencies["@nuxt/schema"], .devDependencies.nuxt, .devDependencies["@nuxt/test-utils"]' packages/nuxt/package.json jq -r '.resolutions?"@nuxt/kit"' package.json 2>/dev/null || echo "no root resolutions for @nuxt/kit"packages/nuxt/playground/pages/index.vue (1)
6-8
: Use or remove unused store calls to avoid lints/tree-shaking surprises.Either assign them or comment them out.
-useTestStore() // ~/domain/one/stores/testStore.ts -useSomeStoreStore() // ~/stores/nested/some-stores.ts -useBasicStore() // ~~/layers/layer-domain/stores/basic.ts +// const test = useTestStore() // ~/domain/one/stores/testStore.ts +// const some = useSomeStoreStore() // ~/stores/nested/some-stores.ts +// const basic = useBasicStore() // ~~/layers/layer-domain/stores/basic.ts
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
package.json
(0 hunks)packages/nuxt/package.json
(1 hunks)packages/nuxt/playground/layers/layer-domain/nuxt.config.ts
(1 hunks)packages/nuxt/playground/layers/layer-domain/stores/basic.ts
(1 hunks)packages/nuxt/playground/pages/index.vue
(1 hunks)packages/nuxt/src/module.ts
(2 hunks)
💤 Files with no reviewable changes (1)
- package.json
🔇 Additional comments (3)
packages/nuxt/playground/layers/layer-domain/nuxt.config.ts (1)
1-1
: LGTM; ensure the layer is actually extended by the playground app.Add this layer to playground/nuxt.config.{ts,js} via extends so the demo exercises the new behavior.
#!/bin/bash rg -nP --glob 'packages/nuxt/playground/nuxt.config.*' -C2 '(?<=extends:\s)(\[|\')' sed -n '1,120p' packages/nuxt/playground/nuxt.config.ts 2>/dev/null || truepackages/nuxt/src/module.ts (1)
10-11
: Importing getLayerDirectories is correct for this use.API exists in Nuxt Kit ≥3.19 and exposes layer.app/root for path composition. (dev.nuxt.com, newreleases.io)
packages/nuxt/playground/layers/layer-domain/stores/basic.ts (1)
1-5
: LGTM.Simple store suffices to validate auto-import resolution across layers.
commit: |
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
The PR fulfills these requirements:
fix #xxx[,#xxx]
, where "xxx" is the issue number)Other information:
getLayerDirectories
so quickly@nuxt/kit
was globally overridden in the monorepo workspace package.json. I'm not entirely sure why that was there in the first place, so that needs some extra review caution 🙏🏻Fixes #3028
Summary by CodeRabbit
New Features
Chores