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

Loss of reactivity upon assignment in the condition #3512

Closed
PieceOfGood opened this issue Sep 5, 2019 · 4 comments · Fixed by #3533
Closed

Loss of reactivity upon assignment in the condition #3512

PieceOfGood opened this issue Sep 5, 2019 · 4 comments · Fixed by #3533
Labels

Comments

@PieceOfGood
Copy link

To Reproduce

<script>
    let toggle = false;

    function _toggleMe() {
        if (toggle = !toggle) {
            console.log(toggle);
        } else {
            console.log(toggle);
        }
    }
</script>

<button on:click={_toggleMe}>Click</button>
<span class="text {toggle ? "red" : "blue"}">
    text
</span>

Describe the bug
If assign value to toggle in the condition, toggle classes happens only once.

Expected behavior
But, if assignment take out to before the compare, it worked, as it should:

<script>
    let toggle = false;

    function _toggleMe() {
        toggle = !toggle;
        if (toggle) {
            console.log(toggle);
        } else {
            console.log(toggle);
        }
    }
</script>

<button on:click={_toggleMe}>Click</button>
<span class="text {toggle ? "red" : "blue"}">
    text
</span>

Information about your Svelte project:

  • Chrome 76

  • OS Windows 10

  • Svelte version 3.9.2

  • Rollup

Severity
Simple problem. Doesn't affect something important.

@hbendev
Copy link

hbendev commented Sep 7, 2019

Indeed, the compiled code doesn't have the prop invalidation in the best place:
function _toggleMe() { if (toggle = !toggle) { console.log(toggle); $$invalidate('toggle', toggle); } else { console.log(toggle); } }

An easy solution would be to generate invalidation into the condition itself, like so:
if($$invalidate('toggle', toggle = !toggle))
And make the function added to the instance in runtime/internal/Component.ts return its second param. I don't think it would cause any trouble, also an edge case would be removed from parsing.

@Conduitry
Copy link
Member

@mrkishi mentioned in the context of another issue the idea of making $$invalidate return its second argument. It does seem like it would make a few things simpler.

@Conduitry Conduitry added the bug label Sep 7, 2019
@hbendev
Copy link

hbendev commented Sep 7, 2019

I made it. I was wondering if this should apply to function calls too, but for me that doesn't seem to fit into Svelte's current "best practices" (eg.: f(x += 4)).
I mean, if someone wanted to use a simple variable, just use a pure assignment operator somewhere, that's what this framework is all about anyway. If one wants to calculate complex stuffs, do it inside the function itself.

@hbendev hbendev mentioned this issue Sep 7, 2019
4 tasks
Rich-Harris added a commit that referenced this issue Sep 8, 2019
Rich-Harris added a commit that referenced this issue Sep 8, 2019
@Rich-Harris
Copy link
Member

fixed in 3.10.1 — thanks

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