QwikDev / qwik

Instant-loading web apps, without effort
https://qwik.dev
MIT License
20.7k stars 1.29k forks source link

[🐞]Store array update does not rerender list #5662

Open Domniq opened 9 months ago

Domniq commented 9 months ago

Which component is affected?

Qwik Runtime

Describe the bug

There is a problem with rendering updated values using list { store.array.map } I can change values only one or twice, depending on code usage after that any further modification is not updating values visually, however store values are changed I've created thread on discord https://discord.com/channels/842438759945601056/1191080926554361992 I managed to find a solution and it's working but it should be used this way

As you can see on the image store.values are different than those that are visible using .map problem

As i mentioned earlier 1 or 2 operations are displayed any further only updates store values but not items rendered via { store.array.map }

IMPORTANT: This code is working in stackblitz https://stackblitz.com/edit/qwik-starter-kzdpd9 (version 1.2.11)

Reproduction

https://qwik.builder.io/playground/#v=1.3.1&f=7VjdbtMwFH4VSxNyAnVI0r80I5UGmgSIAdLgapqQUztrWLdEo6Xrqt7s3fZenGMnTtqU%2FYB2MWmbFDmxfc7xd%2F4%2BtxY0ftCteUp5RjfdVi0YWlUv1kPV3O8RV0reV2id7yBqTyDNoW1xgATBSEVY8IMWvGHlKt8B8nsFZsEklDFRxRGWkLmoDMpI2FB%2BdLwaIv64HyTUVh4VoC2VZdQPuOvLfpe1AzdgHd7rMt5ORmzQd92BL%2BRADHzaKuym309vrrkg84WQo3E2X1CywlM9TJ5Xyds7nZ3NJlhotojiXhz3ZdBmXLY56%2FQ9ybgbJ2zkcVAi3F7P6zZMu%2BL5zXUGjGGLwH7cSdpeIFgwij3WEd2EDQYJZ57vxnGvG7STON4icA64XcnzlBuJx%2FBUwyV4TuahboRYJoy7UlH4SlM77fGhRaletC1FkIyWFI0Cm8mhBYUkmcjLXXLCQYvn5jD8OYNenywYNiaIFIimHDgQi%2BV0LuX5LhCkSzZPxXQckq6LO%2BjKJMotGvDJRHqhGzVQ32wyOwNxSnMHFedcCDhESHxlB7QjyAawKr8k0MXhwDuijf%2FlFLvgIp1ByKndFY8ByycZbNxJkmQX6IUy1XPdFzVLsVz4wz0hkO5AKfBrE6kiwJhzEcX%2BQAHsiIJxP9BzlMCpRnKcYa5GtEwKaqoFFhPF8ityu9TJBuoslWb2ClWb24ApM3hLeKJQ7ot0ei8cJSz8XyAP%2BW%2BpkWyBTEATP9wCZzE02aq2OlXVcuDSYVnF%2B6KlWJ1SpPPH%2BONULqKlml09biIpjfmwLMELJxUr5Apr001Y1L2iOofGphKCqK%2BLWQs524aGUXxX90p9RNq0c7j8ePjls6OLDhyxjOo3r3HfZqPEimR4hMmBkNzVYDS90OVOMfIIGLQmfw5w7f2JxOHbxQdhVdlpA18m778dfFLX2GKNKolpQixc4gD5nMHFVjP7IXG1nlJTiVbTJBKpZXqxbrzWAZ%2BOnWSSZRd6iFef7Azc8ZJYkCfqjzBMGRy8Ip5t41O92s60ZEt22Ul0b6jMVJ8LtzSiNp%2F9GhtvA8qlAL0V4KLAIXA7CDAO2AiS5jExbMKCvZR95SGuqBL8L77YMHItPLWnRHMyFUgha8cwJeDuQHr0Ez1SdJU4%2FFNwwE8R%2B3w0hrIGxzcFzYgHc0cAKomiNZBtUxo2BR4pIcc1n%2BiAK1jJ1shbc6UJxuerztO76vwB

Steps to reproduce

Here are different ways to modify store.arrays: // Not working #1 (can modify 1 element) const category: PartCategoryData = { id: id.value, name: name.value }

store.categories.forEach((c, index) => {
  if (c.id === category.id)
    store.categories[index] = category;
});

// Not working #2 (can modify 1 element)
store.categories = store.categories.map((category) => {
  if (category.id === id.value) {
    return {
      ...category, name: name.value
    };
  }

  return category;
});

// Not working #3 (can modify 1 element)
const modified = store.categories.map((category) => {
  if (category.id === id.value) {
    return {
      ...category, name: name.value
    };
  }

  return category;
});

store.categories.splice(0, store.categories.length, ...modified);

// Not working #4 (can modify 2 elements)
const selectedCategory = store.categories.find(category => category.id === id.value)

if (selectedCategory) {
  selectedCategory.name = name.value;
}

// Not working #5 (can modify 2 elements)
store.categories.forEach(category => {
  if (category.id === id.value) {
    category.name = name.value;
  }
});

// Not working #6 (can modify 2 elements)
for (const category of store.categories) {
  if (category.id === id.value) {
    category.name = name.value;
    break; // If you only want to modify the first matching item
  }
}

// Not working #7 (can modify 1 element)
store.categories = store.categories
.filter(category => !(category.id === id.value))
.concat(store.categories.filter(category => category.id === id.value)
  .map(category => ({ ...category, name: name.value }))
);

System Info

Windows 11, Opera browser, mozilla, edge
In my project im using Qwik ^1.2.15

Additional Information

You can find more on discord thread https://discord.com/channels/842438759945601056/1191080926554361992

wmertens commented 9 months ago

Did Qwik 1.3.2 fix anything?

Domniq commented 9 months ago

Nah, still an issue image

I managed to find workaround by clearing store and assigning it once again after 'empty' await image

Without await Promise it does not update displayed items, only store gets updated This version if working fully https://stackblitz.com/edit/qwik-starter-kzdpd9

wmertens commented 9 months ago

It seems that 1.2.13 works correct, right?

Domniq commented 9 months ago

Stackblitz link I've provided uses "@builder.io/qwik": "^1.2.11", "@builder.io/qwik-city": "^1.2.11", It working fine there

Domniq commented 9 months ago

I did some test on qwik@1.2.11 and it's not working there. So only in Stackblitz with qwik 1.2.11 it's working, might it be environment issue?

wmertens commented 9 months ago

Very odd. Ideally we'd have a minimal e2e test case but if it even differs across environments...

Domniq commented 9 months ago

My bad, i've provided wrong link for stackblitz, here is correct https://stackblitz.com/edit/qwik-starter-kzdpd9, previous version was modified. I downloaded this stackblitz repo and run it on localhost, now it is working on 1.2.11

wmertens commented 9 months ago

Ok in that case the playground is equally valid.

Domniq commented 9 months ago

Yes, currently im using Code Compare for file difference with node_modules because everything matches except node_modules folder npm create qwik@1.2.11 and downloaded stackblitz project are compared

Domniq commented 9 months ago

package-lock.json differs https://www.diffchecker.com/TwlJfa9s/ On the left npm create qwik@1.2.11, on the right taken from stackblitz

wmertens commented 8 months ago

Can you make a minimal playground repro that always triggers the bug? I'm having trouble reproducing it.

Domniq commented 8 months ago

Here is minimalistic version with several solutions to save updated values in store Quick descripiton:

I've tested it few time on 1.3.5 qwik.builder.io/playground removing - working adding - working editing - works only for 1 / 2 operations of changing value of element (value in store.categories is updated, but rendered items via object.map isn't)

In my project I have found that sometimes wrong index / key is removed

Here is link for code: https://pastebin.com/4MiRegHd Or download zip with index.tsx from attachments index.zip

the-zimmermann commented 8 months ago

@Domniq isn't the deep option on useStore change this behavior?

wmertens commented 8 months ago

@Domniq can I ask you to make the repro smaller, in the playground?

Ideally, you make it so that only a few clicks are needed to trigger the problem, and the app should be as small as possible, no styling or nice content. Just exactly what is needed to trigger.

Here's the playground link to start from: link

Domniq commented 8 months ago

@the-zimmermann it does not change anything

Here is smaller repo https://qwik.builder.io/playground/#v=1.4.0&f=7VfdbtMwFL7nKbwrJ9BESbv%2BpUsQg0kMMUCUu2lCTu2sYd1Stenaquq7c44dx0nLVqZpF0jsoosT%2B%2Fx%2B55zPFdA0%2B9VMycyooduogKFhZrF6xFksn2S%2FewxhRIdEiv4GU%2FQ9APgaKh4mGIPoYFxSHhRUoQErbGJ6DdEvJTwK0oJVSNYRGr6wgSpGbdBSgj3tl1fbCHOB50FCZedlEcCNNI02e8xrim7bafW8nnPMOm2HtZKR0%2B96Xr%2FJRZ%2F3m7RRGE7PVmO2mOeUbNGbp4nxjZhTlsMYX%2B%2BLYX4cd0Wv5TDRYs5x1xcO8%2BLEGfkMFHCv0%2FHbRsz59V2KoN%2BX042Pk5bf405vFPvOMW8nTr%2BfMMdvenHcafdaSRwbOcPFfCru5lVJV%2FC7bZANF2IayJnXAIgzqLt7OIFrbBCkzE7Ki9QoVqcyHFmUFrseKA%2BgoJqgUeAyUxhAAUkmYjUgvyDOabJ2cBYBIAA1U6A9TizypRB3A%2BBEK2eZ8nwckLbnTVcDuq3UCnLbcgXrVBJIBGpIsb9SsDikjPOfGANKQPNIjDNEeEg1kGhZbaocJU829HCjIPqOc0uC095G8FzyaWOMZNpPtU3wNH%2B%2BcUN2L5R1DZAKFuKLAybqRQkped41ReQCH7asYr1GPTXnyI1Yh7o6127Kty%2BSY1A2LQTTUTbJZgGgjA%2FIaDGb40LSKjEb0L2gfBe3mQlLxVCIT81uHIWP6bueSTMPaxxKdl2GrMiF0YV5rmurZcTGVqltiE4VRwWKBfW4AKJ5EkfSF8hrBNSMfBp%2B%2FeKqmQfRLcBZkX6CN7RoI7epSjXbwAr5dWewyDIuB%2B%2BOO%2FvtFx0Mimmjm4GazKpdSDIbAvlUvMkFmno2Efh4uj7nVgX8Njr08cfFZ3kFLDaBMVKEC5wN2HhocohvVcdJ%2Bf7XlOPJ6jVS75JkoOphWTcBOTRoXt5ZdCchlnHZLe4AEfGUJq1rtVqBqt2CRa8JgYsgsdQ2HRGSJXhEy1BqKuEiYRiWkTS7yomqAg4ajWlKV22PHA5aivkew0C50UuIvvqV%2F3cdgPNgJu7dQp5eqetHmaxqPf9tulSelEvo9NFSknYX4pOks1uLfsjIOluQJYPbY55Ba0EdcNmDssML11tq29JUNdlUkv5g9l7vTNIJ9Ih6%2B6yF6kiGfBeQ5ZQ57OBTsFdOwOdD7%2BFuQEK5rcQYhN%2B6YPnYTSZZNlOPeD%2FPbqFbviaW76k%2F4hD1SN4Q37bxVy5tN9eU3tbER1EZY%2BYhOF26LvS%2B%2BmszC64ULGs9hiLllfgj%2Fy9G%2F97F6Dc

Here is video for you to see what's wrong https://www.youtube.com/watch?v=curY0UAvcLg

gioboa commented 6 months ago

It's really strange because I can't reproduce the problem constantly. I'll add a specific test in the Qwik v2 test just in case.

gioboa commented 6 months ago

This example always fails :eyes:

import { component$, useStore } from '@builder.io/qwik';

export default component$(() => {
  const store = useStore<{ users: { name: string }[] }>(
    { users: [{ name: 'Misko' }] },
    { deep: true, reactive: true }
  );

  return (
    <>
      {store.users.map((user, key) => (
        <div key={key}>
          <div
            onClick$={() => {
              store.users = store.users.map(({ name }: { name: string }) => ({
                name: name === user.name ? name + '!' : name,
              }));
            }}
          >
            {user.name}
          </div>
        </div>
      ))}
      <span>{JSON.stringify(store)}</span>
    </>
  );
});
gioboa commented 6 months ago

You found a workaround and that's okayish but with Qwik v2 it's working fine. @Domniq thanks for opening this issue because it helps us to improve the codebase.

Domniq commented 6 months ago

You found a workaround and that's okayish but with Qwik v2 it's working fine. @Domniq thanks for opening this issue because it helps us to improve the codebase.

In most cases that workaround does the job well, sometimes it does not and i have to press button twice in order to "refresh" (switch tabs that filters "store" by status (int), stored array Type has 7 fields with various types)

Qwik v2 - you mean upcoming major update?

gioboa commented 6 months ago

Yep, the new version of the rendering engine. This change will not affect final users, you can check the build/v2 branch if you are curious.

Domniq commented 6 months ago

Great news! :)

mhevery commented 6 months ago

ROOT CAUSE ANALYSIS: This is a bug in Optimizer. The optimizer incorrectly marks the onClick listener as 'const'/'immutable'. Because it is const, the QRL associated with the click handler always points to the original object, and it is not updated.

wmertens commented 6 months ago

@mhevery so this is also a problem in V2?

jahwin commented 2 months ago

Hey @mhevery and @gioboa,

I encountered the same issue. When running this code:

import { component$, useStore } from '@builder.io/qwik';

export default component$(() => {
  const tasks = useStore<{ label: string; prio: number }[]>([]);

  return (
    <>
      <div>
        <p>
          <button
            onClick$={() => {
              tasks.push({ label: 'New Task' + tasks.length, prio: 1 });
            }}
          >
            Add new Task
          </button>
        </p>
        <ul>
          {tasks.map((task, index) => (
            <li key={index} class="flex">
              <span>{task.label}</span>
              <span>=====</span>
              <button
                onClick$={() => {
                  tasks.splice(index, 1);
                }}
              >
                Remove
              </button>
            </li>
          ))}
        </ul>
      </div>
    </>
  );
});

the removal of elements is not working.

I tested this on versions from 1.0.0 to 1.2.14, and it worked fine. However, from version 1.3.0 to 1.7.2, it's not working.

gioboa commented 2 months ago

I see, thanks for your detailed information. @Varixo do we have a test case for this in v2?

wmertens commented 2 months ago

It doesn't seem to be the optimizer, the code and html between versions remains the same. Something in 1.3 incorrectly removes nodes.

Here's an illustration of the bug:

jahwin commented 1 month ago

I have continued testing and would like to update you on my findings. In a previous message, I mentioned that versions from 1.0.0 to 1.2.14 worked fine, but versions 1.3.0 to 1.7.2 did not. However, I have discovered that even in versions 1.0.0 to 1.2.14, including a component reintroduces the issue.

Here are the steps to reproduce the problem:

  1. Add a few tasks.
  2. Remove one task in the middle.
  3. Notice that the task at the end appears to have been removed.
  4. Click the rerender button.
  5. Observe that the task in the middle was actually removed.

Below is the relevant code snippet:

import { component$, useStore, useSignal } from '@builder.io/qwik';

export const TaskItem = component$((props: { name: string }) => {
  return <p>Hello {props.name}</p>;
});

export const Counter = component$(() => {
  const rerender = useSignal(0);
  const tasks = useStore<{ label: string; key: number }[]>([]);

  return (
    <>
      <div key={rerender.value}>
        <p>
          <button
            onClick$={() => {
              tasks.push({ label: 'New Task' + tasks.length, key: 1 });
            }}
          >
            Add new Task
          </button>
          <button
            onClick$={() => rerender.value++}
          >
            Rerender
          </button>
        </p>
        <ul>
          {tasks.map((task, index) => (
            <li key={task.key} class="flex">
            <TaskItem name={task.label} /> 
             {/* <span>{task.label}</span> */}
              <span>&nbsp;</span>
              <button
                onClick$={() => {
                  tasks.splice(index, 1);
                }}
              >
                Remove
              </button>
            </li>
          ))}
        </ul>
      </div>
    </>
  );
});

@mhevery , @wmertens , @gioboa , could you please address this issue in the next release (version 1.7.4)? We have two significant projects built using Qwik, and switching frameworks is not a feasible option for us. Your prompt attention to this matter would be greatly appreciated. Thank you.