Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic properties in transition "in" directive not working #6505

Closed
sidharthv96 opened this issue Jul 7, 2021 · 4 comments · Fixed by #6516
Closed

Dynamic properties in transition "in" directive not working #6505

sidharthv96 opened this issue Jul 7, 2021 · 4 comments · Fixed by #6516
Labels

Comments

@sidharthv96
Copy link

sidharthv96 commented Jul 7, 2021

Describe the bug

When the params passed to "in" transition is changed dynamically, the value is not reflected.
The values passed to "out" work fine.

Reproduction

https://svelte.dev/repl/ca9e1328264c4f3395d72d786c9aa113?version=3.38.3

Logs

No response

System Info

REPL

Severity

annoyance

intro.bug.mov
@tanhauhau tanhauhau added the bug label Jul 8, 2021
@baseballyama
Copy link
Member

@tanhauhau

Preface

I would like to contribute to Svelte because this project is really fun!👍
And I want to start with bug fixes.

Now I'm trying to solve this issue.
But before create PR, I want to confirm that my way is correct or not.

Explain About This Issue

When in transition event occurred,
below code runs.

bundle.js

add_render_callback(() => {
	if (div_outro) div_outro.end(1);
	if (!div_intro) div_intro = create_in_transition(div, foo, {});
	div_intro.start();
});

According to this code, create_in_transition runs only if div_intro is not initialized.
But according to this issue, programmers can manage in:fly params if they use state or local variable or something.
It means runtime can't judge that create_in_transition is necessary or not at all times.

So my opinion is that remove if (!div_intro) for calling create_in_transition.

How To Solve This Issue

And for fix this issue, I need to change the compiler like this.

svelte/src/compiler/compile/render_dom/wrappers/Element/index.ts

Before Fix (Line 755)

@add_render_callback(() => {
	if (${outro_name}) ${outro_name}.end(1);
	if (!${intro_name}) ${intro_name} = @create_in_transition(${this.var}, ${fn}, ${snippet});
	${intro_name}.start();
});

After Fix (Line 755)

@add_render_callback(() => {
	if (${outro_name}) ${outro_name}.end(1);
	${intro_name} = @create_in_transition(${this.var}, ${fn}, ${snippet});
	${intro_name}.start();
});

Thank you for reading.
And I hope you will tell me what should I do next!🧐

@tanhauhau
Copy link
Member

sounds about right,

  1. usually i would searched through git blames on that line of code
if (!${intro_name}) ${intro_name} = @create_in_transition(${this.var}, ${fn}, ${snippet});

whether it was always there since beginning, or was it added later on with some reason

  1. I would write a test that would fail before the fix, and passed after the fix

@baseballyama
Copy link
Member

Thank you for the reply quickly and guide me!🙏

usually i would searched through git blames on that line of code

Sure! I will check that why the IF statement is there.

I would write a test that would fail before the fix, and passed after the fix

Sure! I will follow the contribution guide.

Then I will create PR if there was nothing special.

@Conduitry
Copy link
Member

This should be working now in 3.40.3 - https://svelte.dev/repl/ca9e1328264c4f3395d72d786c9aa113?version=3.40.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants