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

SvelteSet: Array.from(set) contains X within it while set.has(X) returns false #14827

Closed
gyzerok opened this issue Dec 24, 2024 · 9 comments
Closed
Labels
awaiting submitter needs a reproduction, or clarification

Comments

@gyzerok
Copy link

gyzerok commented Dec 24, 2024

Describe the bug

In our project we have following code

class Foo {
  itemIds = new SvelteSet()
  significantId = $state(<id>)

  items = $derived.by(() => {
    return Array.from(this.itemIds).map(id => <map-id-to-item>)
  })

  get bar() {
    if (this.itemIds.has(this.significantId) {
      return null
    }

    return <something-else>
  }
}

Recently (maybe around a week ago) we've started getting occasional bugs in UI. They are really sporadic, but when I caught one myself I've found that when I do foo.itemIds in console, I see significantId in there, but if I call foo.itemIds.has(foo.significantId) then it returns false. Then leads to this.bar being calculated incorrectly and hence UI bug.

Reproduction

It seems like this bug is happening quite rarely and so far I have no idea how even to approach reproduction. Would gladly accept any advice on how to arrive there.

Logs

No response

System Info

svelte 5.15.0

Severity

annoyance

@Conduitry Conduitry added the awaiting submitter needs a reproduction, or clarification label Dec 24, 2024
@gyzerok
Copy link
Author

gyzerok commented Dec 24, 2024

Looking at the code for SvelteSet I see 2 things

In .keys() implementation it does not read .#version

keys() {
return this.values();
}

And in [Symbol.iterator] implementation it uses .keys() instead of .values()

[Symbol.iterator]() {
return this.keys();
}

Could it be that that's the possible reason for the bug?

@gyzerok
Copy link
Author

gyzerok commented Dec 24, 2024

Also we've started experiencing this issue around 1-1.5 weeks ago. Since we are updating to the new versions swiftly it might have been some changes, that started triggering this problem.

From what I can see somewhat related to reactivity are these 2 changes: #14724 and #14738

@paoloricciuti
Copy link
Member

Keys just call values that read the version so that's not the problem. We do need a reproduction because we can't know how you are using this or what the actual code is (maybe you are calling something else which is the problem).

The ideal would be a minimal reproduction and that would mean you should start from your code, try to understand with which input it fail, and try to make the minimal version of your code that still make the bug error (without any business logic from your part). But if you can't at least a repl with your code and a step to reproduce is kinda required for us to even begin to take a look

@gyzerok
Copy link
Author

gyzerok commented Dec 24, 2024

@paoloricciuti yeah, I get it, but don't even have a clue as to what I need to do to reproduce it in our codebase, less so about how to minimize it.

From what I can see somewhat related to reactivity are these 2 changes: #14724 and #14738

Can you, please, take a look at the PRs I've mentioned here and just take a guess if they could have affected it somehow? I know I am asking for needle in the haystack, but even slightest tip would be helpful.

Or maybe there is something I can check within the SvelteSet instance inside my console next time I catch this bug? Now I am thinking I could have checked set.size to see if that equals Array.from(set).length. But maybe something else?

@paoloricciuti
Copy link
Member

The point is: it could be...but if we don't have the full picture of how you use it and the exact code could be something completely different and we might spend hours trying to find this issue while the solution might have been obvious with an actual reproduction.

I would say the very minimum would be to have the actual code and all the instances on how it's used (that would still be kinda of mess but at least possibly doable)

@gyzerok
Copy link
Author

gyzerok commented Dec 24, 2024

It seems like we found what the root issue was and it's indeed not related to svelte. Apologies for taking your time

@gyzerok gyzerok closed this as completed Dec 24, 2024
@paoloricciuti
Copy link
Member

It seems like we found what the root issue was and it's indeed not related to svelte. Apologies for taking your time

What it was (I'm a curious cat 😺)

@gyzerok
Copy link
Author

gyzerok commented Dec 24, 2024

We were receiving int64 number in json instead of int64 number converted to string. It'd be difficult to explain how everything was affected exactly. The funny thing here is that since we are a real time application we are receiving updates. So while I was looking at the set and running some checks in the console I've received correct data. So I was checking old values against new values and that's why the mismatch.

Fortunately right after I wrote my last comment my coworker also got the same bug and we were able to investigate it again in his console :)

@paoloricciuti
Copy link
Member

Glad it worked out (and props to you for working on Christmas Eve 😁)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting submitter needs a reproduction, or clarification
Projects
None yet
Development

No branches or pull requests

3 participants