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

Data or Store references are not updated correctly inside directive #1793

Closed
hrj opened this issue Oct 23, 2018 · 10 comments · Fixed by #1797
Closed

Data or Store references are not updated correctly inside directive #1793

hrj opened this issue Oct 23, 2018 · 10 comments · Fixed by #1797

Comments

@hrj
Copy link

hrj commented Oct 23, 2018

Consider the following example:

<ul>
{#each plans as plan, idx}
  <li on:click="changePlan(idx)" class:selected="planIdx == idx">{plan.title}</li>
{/each}
</ul>

<script>
  export default {
    data() {
      return {
        planIdx: 0,
        plans: [{ title: "Plan 1" }, { title: "Plan 2" }]
      };
    },
    methods: {
      changePlan(idx) {
        console.log("changing plan to", idx);
        this.set({planIdx: idx});
      }
    }
  }
</script>

<style>
.selected {
  font-weight: bold;
}
</style>

After clicking on Plan2, it should be highlighted, but it is not. Tested with 2.13.15 and 2.14.2.

The above example uses data but the same bug is found with store as well.

If one of the data elements is part of the rendered HTML, then everything works fine. For example, if we change the HTML above to:

<li on:click="changePlan(idx)" class:selected="planIdx == idx">{plan.title} {planIdx}</li>

then everything works fine.

@hrj hrj changed the title Data or Store is not updated correctly inside directive Data or Store references are not updated correctly inside directive Oct 23, 2018
@PaulMaly
Copy link
Contributor

@hrj Hi, please, use “keyed each” for quick workaround. I’m not sure is this correct behavior or not.

@mrkishi
Copy link
Member

mrkishi commented Oct 23, 2018

I don't believe this is correct behavior. Keyed or not, the final DOM should be the same.

The difference between the keyed and non-keyed version is that the non-keyed will just reuse whatever's already on the DOM regardless of its current state.

@PaulMaly
Copy link
Contributor

@mrkishi Yes, agree. Seems it's "class"-directive specific issue. @jacwright What do you think?

@hrj
Copy link
Author

hrj commented Oct 24, 2018

@PaulMaly It's not specific to class directive. In my project, I encountered the issue in an on:click directive.

@stalkerg
Copy link
Contributor

can somebody make REPL?

@hrj
Copy link
Author

hrj commented Oct 24, 2018

@stalkerg The example can be tested when copy-pasted to REPL. (I didn't save it there because REPL asks for too many Github permissions like "read and write secret gists".)

@Rich-Harris
Copy link
Member

The each block doesn't recognise planIdx as a dependency, only plans. Working on a fix

@Rich-Harris
Copy link
Member

Fixed in 2.14.3

@hrj
Copy link
Author

hrj commented Oct 24, 2018

🎉
Thanks; I tested that the fix works even for dependencies in on:click directive.

@PaulMaly
Copy link
Contributor

@hrj I meant that you still were able to switch classes without class-directive.

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

Successfully merging a pull request may close this issue.

5 participants