solid icon indicating copy to clipboard operation
solid copied to clipboard

Objects created in a different window are not reactively wrapped by store

Open katywings opened this issue 1 year ago • 3 comments

Describe the bug

If you share a store setter from an iframe to its parent and call it via the parent, objects assigned to the store via this method, will not be reactive.

Your Example Website or App

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

Steps to Reproduce the Bug or Issue

  1. Share a store setter from an iframe with its parent window
  2. Listen to all values inside of the store, e.g. with createEffect(() => console.log(JSON.stringify(state))
  3. Via the parent window, assign a new object to the store
  4. Via the parent window, update a value inside of this new object
  5. Notice how this change will not trigger a console.log

Expected behavior

Objects assigned to a store via a different window should become reactive like any other object.

Screenshots or Videos

No response

Platform

  • OS: [Linux]
  • Browser: [Chrome, Firefox]

Additional context

The reason why objects assigned this way will not be reactive, is the prototype check in https://github.com/solidjs/solid/blob/4d824b08d8534d2a079f9ca4c1ea980684c05582/packages/solid/store/src/store.ts#L75. Different windows have different Object.prototype and therefore an object assigned via another window, will not be considered as wrappable by solid store.

Since the prototype check works as intended for normal non-iframe use cases, a sensible way would be to just allow a hook into the check, e.g. something like this:

const wrappablePrototypes = [Object.prototype];
export const registerWrappablePrototype = (p) => wrappablePrototypes.push(p);

export function isWrappable(obj: any) {
  let proto;
  return (
    obj != null &&
    typeof obj === "object" &&
    (obj[$PROXY] ||
      !(proto = Object.getPrototypeOf(obj)) ||
      wrappablePrototypes.indexOf(proto) >= 0 ||
      Array.isArray(obj))
  );
}

registerWrappablePrototype would allow advanced developers to add parent/iframe Object.prototype, vice-versa.


Edit: Alternative: how about we use duck typing to figure out if proto is wrappable, if it quacks like a Object.prototype it must be one. We would just have to make sure that the duck typing works across realms.

katywings avatar Nov 16 '24 13:11 katywings

Yeah this is one of those annoying things. Forcing this into user space seems really awkward. We need to consider the general solution. This check is to prevent classes. I'm deep into rethinking stores right now. Maybe that constraint is not necessary. It is worth investigation.

ryansolid avatar Nov 19 '24 17:11 ryansolid

I also looked a bit deeper regarding the runtime typing approach. Most isPlainobject solutions out there sadly just do it with Object.prototype - (wheres the iframe love 🤣?), but Lodash has a different solution, that works across realms from what I found (https://github.com/lodash/lodash/blob/4.17.15/lodash.js#L12045): Their Ctor instanceof Ctor check filters out classes and funcToString.call(Ctor) == objectCtorString takes care of the rest. I wonder what this would mean for performance though 🤔..

katywings avatar Nov 20 '24 15:11 katywings

@ryansolid Is there anything I can do to speed up the solution-finding for this issue 🙂? Not gonna lie, being able to synchronize stores between realms without reconcile would be an absolute game changer for Nitropage, it would improve performance a lot and get rid of many sync timing issues.

katywings avatar Nov 28 '24 20:11 katywings