Skip to content

Conversation

@hackape
Copy link
Contributor

@hackape hackape commented Jun 28, 2023

fix #8233

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with feat:, fix:, chore:, or docs:.
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests and linting

  • Run the tests with pnpm test and lint the project with pnpm lint
@hackape
Copy link
Contributor Author

hackape commented Jun 28, 2023

As of v4.0.0, this bug behaves a lil bit differently, but still persists. REPL repro in v4.0.0.

<script> import {scale} from "svelte/transition"; let show = true; let tag = "div" </script> <label>	<input type='checkbox' bind:checked={show} /> Show </label> {#if show} <!-- uncomment below element and transition is also applied to svelte:element --> <!-- <div transition:scale /> -->	<svelte:element this={tag} transition:scale>Foo</svelte:element> {/if}

Currently when <svelte:element this={tag} transition:foo /> is the only local-transition'ed child to a parent block, that block's fragment will NOT have frag.i(local) or frag.o(local) methods.

If transition is global, or if parent block has other non-dynamic elements with transition, the bug can not reproduce.

@hackape
Copy link
Contributor Author

hackape commented Jun 28, 2023

Analysis

<svelte:element />'s rendering is delegated to .child_dynamic_element, and it's done inside .child_dynamic_element_block instead of the original parent block.

The early return at L223 prevents parent block's ctor codepath to reach:

if (node.intro || node.outro) {
if (node.intro) block.add_intro(node.intro.is_local);
if (node.outro) block.add_outro(node.outro.is_local);

Current logic, when creating .child_dynamic_element inside .child_dynamic_element_block, correctly calls block.add_intro(node.is_local). But this only marks child_dynamic_element_block.has_intros, not the parent block.

If transition is global, the .has_intros marking can bubble up from child_dynamic_element_block to parent. But it cannot bubble when transition is local.

if (!local && this.parent) this.parent.add_intro();

If parent block has other non-dynamic elements with transition, those siblings will help call parent_block.add_intro and cover up the bug.

Solution

There're 2 ways to approach the problem. One is my current solution in this PR. Other is to do in in Block.js:

add_intro(local) { this.has_intros = this.has_intro_method = true; if (!local && this.parent) this.parent.add_intro(); + if (this.parent && this.type == 'child_dynamic_element') this.parent.add_intro(local); }
Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@dummdidumm dummdidumm merged commit 876f894 into sveltejs:master Jun 28, 2023
@hackape hackape deleted the gh-8233 branch June 28, 2023 11:56
@github-actions github-actions bot mentioned this pull request Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants