fix(nuxt): avoid transforming NuxtTeleportIslandComponent with nuxt-client directive#34817
fix(nuxt): avoid transforming NuxtTeleportIslandComponent with nuxt-client directive#34817
Conversation
|
|
@nuxt/kit
@nuxt/nitro-server
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/nuxt/src/components/plugins/islands-transform.ts (1)
101-103: Please add a regression test for pre-wrapped island nodes.Add a case in
packages/nuxt/test/island-transform.test.tswhere the input already containsNuxtTeleportIslandComponent, and assert the transform does not nest another wrapper. This will lock in the fix behaviour.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nuxt/src/components/plugins/islands-transform.ts` around lines 101 - 103, Add a regression test in island-transform.test.ts that feeds the transformer an input already containing the NuxtTeleportIslandComponent wrapper and asserts the islands-transform result does not add a second wrapper (no nested NuxtTeleportIslandComponent). Locate the transformer behavior in islands-transform (the code that checks node.name === 'NuxtTeleportIslandComponent') and write the test to run the same transform function on a pre-wrapped AST/markup and assert the output structure (or snapshot) remains single-wrapped.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/nuxt/src/components/plugins/islands-transform.ts`:
- Around line 101-103: Add a regression test in island-transform.test.ts that
feeds the transformer an input already containing the
NuxtTeleportIslandComponent wrapper and asserts the islands-transform result
does not add a second wrapper (no nested NuxtTeleportIslandComponent). Locate
the transformer behavior in islands-transform (the code that checks node.name
=== 'NuxtTeleportIslandComponent') and write the test to run the same transform
function on a pre-wrapped AST/markup and assert the output structure (or
snapshot) remains single-wrapped.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: eac5b2ad-d5fb-47c6-8a82-2fefb473e03e
📒 Files selected for processing (1)
packages/nuxt/src/components/plugins/islands-transform.ts
Merging this PR will not alter performance
Comparing Footnotes
|
danielroe
left a comment
There was a problem hiding this comment.
this doesn't look like it would work.
maybe add a test to validate the desired behaviour?
huang-julien
left a comment
There was a problem hiding this comment.
LGTM, from what I remember there's also one for teleporting slots
|
Interesting comment, will add a test if it helps you understand my change. So far I did a lot of manual testing (that's why it's still draft), right now if you have in your codebase: <template>
<NuxtTeleportIslandComponent nuxt-client>
<Dynamic>
<NuxtTeleportIslandComponent>
</template>The transformer will do: <template>
<NuxtTeleportIslandComponent nuxt-client>
<NuxtTeleportIslandComponent>
<Dynamic>
</NuxtTeleportIslandComponent>
</NuxtTeleportIslandComponent>
</template>See in the Vite Inspect: Original code: Second step: But if I use the Original code: Second step: With this change, I am almost good to have dynamic markdown rendering support in a server component with hydration for specific components 🚀 |
|
amazing! yes, a test would be great 🙏 |
|
Right now this is a demo of using Comark in a server/island component with the ability to use the CleanShot.2026-04-16.at.17.25.22.mp4I need to do more testing as the behavior changes if the components has slots or not (need to set |
WIP, exploration of Island components with dynamic content on SSR with components.