feat(runtime): improve app.root and app.teleports html tag setup#1726
feat(runtime): improve app.root and app.teleports html tag setup#1726yamachi4416 wants to merge 4 commits into
Conversation
commit: |
yamachi4416
force-pushed
the
improve-root-and-teleport-tag
branch
from
a4e4dac to
24d8ac5
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds configurable Nuxt app root and teleport settings to resolved vitest environment options, refactors window setup to create those DOM nodes through a shared helper, and updates example apps and tests to cover teleport rendering plus custom root and teleport attributes. It also adds a new teleport fixture component and related pages, and enables Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@src/runtime/shared/environment.ts`:
- Around line 109-128: The createElementAndAppend function creates elements
without ensuring a default id is set for teleport elements when teleportAttrs is
undefined or lacks an id attribute. This prevents Vue's Teleport component from
finding the target element. Modify the function to assign a default id of
'teleports' to the created element when no id is provided in the attrs
parameter, ensuring this default is set before the element is appended to the
document body. This should follow the same pattern as the root element which
defaults to 'nuxt-test'.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 10c41009-5eb5-4b24-aaed-fc6af669bc20
📒 Files selected for processing (14)
examples/app-vitest-full/components/TestTeleport.vueexamples/app-vitest-full/pages/other/index.vueexamples/app-vitest-full/pages/other/teleports.vueexamples/app-vitest-full/tests/nuxt/mount-suspended.spec.tsexamples/app-vitest-full/tests/nuxt/render-suspended.spec.tsexamples/app-vitest-workspace/app1/nuxt.config.tsexamples/app-vitest-workspace/app1/test/app.nuxt.spec.tsexamples/app-vitest-workspace/app2/nuxt.config.tsexamples/app-vitest-workspace/app2/test/nuxt/app.spec.tsexamples/app-vitest-workspace/app3/test/nuxt/app.spec.tsexamples/app-vitest-workspace/app3/vitest.config.tssrc/config.tssrc/runtime/shared/environment.tstsconfig.json
| nuxtConfig: { | ||
| runtimeConfig: applyEnv(structuredClone(options.nuxt.options.runtimeConfig), { | ||
| prefix: 'NUXT_', | ||
| env: await setupDotenv(defu(loadNuxtOptions.dotenv, { | ||
| cwd: rootDir, | ||
| fileName: '.env.test', | ||
| })), | ||
| }), | ||
| routeRules: defu( | ||
| {}, | ||
| options.nuxt.options.routeRules, | ||
| options.nuxt.options.nitro?.routeRules, | ||
| ), | ||
| app: { | ||
| rootAttrs: options.nuxt.options.app.rootAttrs, | ||
| rootTag: options.nuxt.options.app.rootTag, | ||
| teleportAttrs: options.nuxt.options.app.teleportAttrs, | ||
| teleportTag: options.nuxt.options.app.teleportTag, | ||
| }, |
There was a problem hiding this comment.
could end users have been dependent on the previous structure? might this be a breaking change?
There was a problem hiding this comment.
Thank you!
I restored the original structure. definitely, it might be a breaking change if any end user is using this.
2 participants
🔗 Linked issue
📚 Description
This PR improves testing setup for html tags for app.root and app.teleport.
<Teleport>target automatically during setup so<Teleport>works by defaultenvironmentOptions.nuxt.rootIdas deprecated, as it requires settingapp.rootIdto the same value andoverrides.app.rootAttrs.idoroverrides.app.rootIdare alternativesAny feedback or thoughts would be appreciated🙏