solid icon indicating copy to clipboard operation
solid copied to clipboard

adds `reconcileWithKeys`

Open titoBouzout opened this issue 1 year ago • 3 comments

Summary

A suggestion to add reconcileWithKeys that @katywings and I have requested in the past. In some situations you want to use reconcile with objects that have keys differently named to id.

       const [target, setStore] = createStore({
        c: {
          a: [
            { ida: 1, name: "1" },
            { ida: 0, name: "0" }
          ],
          b: [
            { idb: 1, name: "1" },
            { idb: 0, name: "0" }
          ]
        }
      });

      const ref = target.c.a[1];

      const source = {
        c: {
          a: [{ ida: 0, name: "0 modified" }],
          b: [{ idb: 0, name: "0 modified" }]
        }
      };

      /*
      setStore("c", reconcile(source.c));
      expect(target.c.a[0]).not.toBe(ref);
      */

      setStore(
        "c",
        reconcileWithKeys(source.c, {
          keys: {
            a: { _key: "ida" },
            b: { _key: "idb" }
          }
        })
      );
      expect(target.c.a[0]).toBe(ref);

      expect(target).toEqual({
        c: {
          a: [{ ida: 0, name: "0 modified" }],
          b: [{ idb: 0, name: "0 modified" }]
        }
      });

How did you test this change?

Hehe, Im running into an issue when trying to write tests for it, when I run npm run test I get this error:

solid-js:test:  FAIL  store/test/modifiers.spec.ts > reconcileWithKeys
solid-js:test: TypeError: Cannot assign to read only property 'Symbol(solid-proxy)' of object '#<Object>'
solid-js:test: ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯

It works in the playground, so I'm lost at to whats going on, any ideas? 🙏🏼 https://playground.solidjs.com/anonymous/9167e413-e0b2-45bf-942a-3ca2153c0416

References

https://discord.com/channels/722131463138705510/780502110772658196/971305532034011156 https://discord.com/channels/722131463138705510/780502110772658196/1278247899616247808

titoBouzout avatar Sep 12 '24 18:09 titoBouzout

⚠️ No Changeset found

Latest commit: fc2d65a7648f35b6992ee28e5843c70c30a46e07

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

changeset-bot[bot] avatar Sep 12 '24 18:09 changeset-bot[bot]

Ah, I see what's going on, it's failing a test (as expected), but then its throwing an unrelated error, that put me off, and I wasn't understanding. I think I can continue writing tests..

titoBouzout avatar Sep 12 '24 18:09 titoBouzout

It's a cool Idea. There have been a few design variations on this. Not completely sure which makes the most sense. Mostly wonder if the changes to Stores in 2.0 will have bigger bearing on what the design ultimately is.

ryansolid avatar Sep 12 '24 20:09 ryansolid