fix: passthrough of query - #2975
#2975fix: passthrough of query#2975serhalp merged 1 commit intonpmx-dev:mainnpmx-dev/npmx.dev:mainfrom johnnyreilly:fix-passthrough-of-queryjohnnyreilly/npmx.dev:fix-passthrough-of-queryCopy head branch name to clipboard
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
📝 WalkthroughSummary by CodeRabbit
Walkthrough
ChangesQuery-aware ISR for
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
nuxt.config.ts (1)
465-472: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid masking the ISR shape with a narrow assertion.
as { expiration: number }disables checking for the newpassQuery/allowQueryfields in this branch, so a typo or unsupported key here will still compile. Please givegetISRConfigan explicit return type, or usesatisfiesagainst the Nuxt ISR type, so these additions stay type-checked. As per coding guidelines, "Ensure you write strictly type-safe code".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nuxt.config.ts` around lines 465 - 472, The `getISRConfig` return shape is being narrowed by the `as { expiration: number }` assertion, which bypasses type-checking for `passQuery`, `allowQuery`, and other ISR fields in this branch. Update `getISRConfig` to use an explicit return type or a `satisfies` check against the Nuxt ISR config type so the `isr` object remains fully type-safe. Keep the existing logic in the `options.fallback` and `extraISR` merge, but remove the narrow assertion so typos or unsupported keys are caught at compile time.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@nuxt.config.ts`:
- Around line 465-472: The `getISRConfig` return shape is being narrowed by the
`as { expiration: number }` assertion, which bypasses type-checking for
`passQuery`, `allowQuery`, and other ISR fields in this branch. Update
`getISRConfig` to use an explicit return type or a `satisfies` check against the
Nuxt ISR config type so the `isr` object remains fully type-safe. Keep the
existing logic in the `options.fallback` and `extraISR` merge, but remove the
narrow assertion so typos or unsupported keys are caught at compile time.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: af9d374c-9485-40ed-a787-8405da3d1a31
📒 Files selected for processing (1)
nuxt.config.ts
|
Hmmm I tried testing with: https://npmxdev-git-fork-johnnyreilly-fix-passthrough-of-query-npmx.vercel.app/ but going here doesn't seem to work: This leads me to think that the fix potentially hasn't worked? If anyone understands nuxt better than me I'd love to hear a suggestion! |
|
Oh wait! It seems to work now! I don't know what's different, but it just worked! Yay! |
🔗 Linked issue
The hope is that this fix will make the change implemented in #2800 work when npmx is deployed. That fix worked locally, but not when live.
🧭 Context
For reasons that I don't fully understand (still a nuxt noob) the query options were not being passed through.
📚 Description
This PR changes the nuxt config in a way that I think will mean it passes through what we need and things should start to work. I hope. I don't know how to test this though. Working locally isn't enough I know; is there a way to push this to a preview environment or similar? Dunno. cc @gameroman