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

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

Open
katywings opened this issue Nov 16, 2024 · 3 comments
Open

Comments

@katywings
Copy link
Contributor

katywings commented Nov 16, 2024

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

proto === Object.prototype ||
. 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.

@ryansolid
Copy link
Member

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.

@katywings
Copy link
Contributor Author

katywings commented Nov 20, 2024

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
Copy link
Contributor Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants