[🐞] V2: Old reactivity issue

[gnemanja]

Which component is affected?

Qwik Runtime

Describe the bug

1. Iterate through an array
2. Read an item's property and pass it as prop (_fnSignal)
3. Use computed in child component won't trigger

Passing the item as a whole OR creating a 150 char long expression works.

I think this reactivity issue is as old as v2 (might help with debugging)

Reproduction

https://stackblitz.com/edit/github-ycxr2gdp?file=src%2Froutes%2Findex.tsx

Steps to reproduce

Run the stackblitz and click on the button. Check browser console for which USE COMPUTED are triggered. You will see that:

1. directlly passing array item -- Works
2. passing item's property -- Doesn't work
3. passing item's property but making a hack expression of 150 characters -- Works

System Info

The version on stackblitz is beta 16, but can be reproduced on many different versions

Additional Information

No response

[sergeche]

Did some research on this topic.

<div>
  {props.items.map(item => {
    // title is wrapped as _fnSignal((p0) => p0.title, [item])
    return <MyComponent title={item.title} />
  })}
</div>

In this case, Qwik expects item to be a reactive primitive to trigger updates in wrapped signal. I guess optimizer assumes that item comes from parent component scope, which is not since it’s a callback argument. So title of child component never updates.

I tried to update example as follows:

<div>
  {props.items.map((_, i) => {
    // title is wrapped as _fnSignal((p0, p1) => p1.items[p0].title, [i, props])
    return <MyComponent title={props.items[i].title} />
  })}
</div>

In this case reactivity works as expected, but it leaks memory: on every parent component update _fnSignal creates new wrapped signal instance which subscribes to props updates, but only most recent signals are used, previous ones are not released. Or maybe it happens for all wrapped signals, even outside callback? Tested it via nested console.log() and heap profiler: after each parent re-render new signals are created, old ones are never released and always update.

IMO, the best solution for this issue is to tell Optimizer to not wrap into signals inside callbacks. I can try to fix that if you will accept this patch. Also, memory leaks seems to be a problem as well. I can investigate further, maybe you can give me some hints about Qwik core where previous signals could be released?

[maiieul]

Small question: did you verify it does work on v1?

[gnemanja]

@maiieul it's not working on v1.16.0
Just made a same example using it:
https://stackblitz.com/edit/github-rjj9bejq?file=src%2Froutes%2Findex.tsx

[Varixo]

@sergeche thanks for the investigation, but the problem is more complex. The _fnSignal is correctly there. The problem is different.

When you look at the generated jsx output you can see:

return _jsxSorted(
  TestComponent,
  null,
  {
    counter: _fnSignal((p0) => p0.attributeInArray.counter, [item]),
    logString: "Nested read through array",
  },
  null,
  3,
  index,
  {
    fileName: "/src/routes/index.tsx",
    lineNumber: 56,
    columnNumber: 13,
  }
);

_jsxSorted params are: type, varProps, constProps and the rest. This means counter prop is incorrectly put in const props while it should be in var props, because has the item as dependency. So it should look like:

return _jsxSorted(
  TestComponent,
  {
    counter: _fnSignal((p0) => p0.attributeInArray.counter, [item]),
  },
  {
    logString: "Nested read through array",
  },
  null,
  3,
  index,
  {
    fileName: "/src/routes/index.tsx",
    lineNumber: 56,
    columnNumber: 13,
  }
);

You can even directly paste it in the code in loop (and import missing things from @qwik.dev/core/internal) and see that it works.
Didn't look at the memory leak, but it could also fix the memory leak.

I am not 100% sure this, however I would like to make changes after my PR #8223 for loop optimizations gets merged to not create unnecessary conflicts, but PR's are welcomed :D

[Varixo]

Should be fixed by #8249, memory leak will be fixed in #8254