fix(nuxt): avoid overwriting multiple head input in island handler #33849
Conversation
|
|
huang-julien
changed the title
huang-julien
changed the title
WalkthroughThe change modifies the head entry merging logic for islands in the runtime handler. The update refines how values are processed during iteration over head entries. When encountering a new value for an existing key containing an array, the new value is now pushed into that array. For non-array values, direct assignment occurs as before. Previously, the code performed direct assignment regardless of the existing value type, which could overwrite array contents. Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip ✨ Issue Enrichment is now available for GitHub issues!CodeRabbit can now help you manage issues more effectively:
Disable automatic issue enrichmentTo disable automatic issue enrichment, add the following to your issue_enrichment:
auto_enrich:
enabled: falseThanks 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 |
![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 (1)
packages/nitro-server/src/runtime/handlers/island.ts (1)
86-90: Fix correctly addresses the overwriting issue.The addition of the
elseclause properly solves the bug where multiple head inputs were being overwritten. Previously, the assignment on line 89 would execute unconditionally after the if block, negating the array push operation. Now, array values are correctly preserved and extended whilst non-array values are assigned only when appropriate.Optional: Consider defensive type checking.
Whilst the current logic should work correctly with proper Unhead usage, consider adding a type check for robustness:
if (Array.isArray(currentValue)) { - currentValue.push(...value) + if (Array.isArray(value)) { + currentValue.push(...value) + } else { + currentValue.push(value) + } } else { islandHead[key as keyof SerializableHead] = value }This guards against edge cases where
valuemight not be an array even whencurrentValueis, preventing potential runtime errors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/nitro-server/src/runtime/handlers/island.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/nitro-server/src/runtime/handlers/island.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx,vue}: Use clear, descriptive variable and function names
Add comments only to explain complex logic or non-obvious implementations
Keep functions focused and manageable (generally under 50 lines), and extract complex logic into separate domain-specific files
Remove code that is not used or needed
Use error handling patterns consistently
Files:
packages/nitro-server/src/runtime/handlers/island.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, webpack, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, default, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, dev, vite-env-api, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, dev, vite-env-api, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, built, rspack, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, rspack, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite-env-api, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite-env-api, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite-env-api, default, manifest-on, json, lts/-1)
- GitHub Check: release-pkg-pr-new
- GitHub Check: test-size
- GitHub Check: typecheck (ubuntu-latest, bundler)
- GitHub Check: typecheck (windows-latest, bundler)
- GitHub Check: test-benchmark
- GitHub Check: code
@nuxt/kit
@nuxt/nitro-server
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
CodSpeed Performance ReportMerging #33849 will not alter performanceComparing Summary
Footnotes |
🔗 Linked issue
fix #33844
📚 Description
This PR fixes a bug where multiple head inputs will result in only having the last input being set in the islandHead Object due to a missing else statement.