mobxjs / mobx

Simple, scalable state management.
http://mobx.js.org
MIT License
27.48k stars 1.77k forks source link

Cannot initialize observable Map/Sets from another window #3892

Closed g6123 closed 3 months ago

g6123 commented 3 months ago

Hi, I'm working on a project that uses MobX to communicate between same-origin iframes and popups.

And I found some issues using observable Maps and Sets across windows (or "realms" according to the ECMAScript spec).

I should admit that this is kinda tricky use-case, but I also found several similar discussions on cross-window cases before: #1644 #2448

Intended outcome:

Can initialize observable maps and sets with a MobX instance from different windows.

For example,

<!-- index.html -->
<iframe id="frame" src="./frame"></iframe>
<script>
  const $frame = document.getElementById('frame');
  $frame.contentWindow.mobx = mobx;
</script>
<!-- frame.html -->
<script>
  const { autorun, makeObservable, observable } = window.mobx;
  const $size = document.getElementById('size');
  const set = observable.set(new Set());
  // ...
</script>

Actual outcome:

Uncaught Error: [MobX] Cannot initialize set from [object Set]

How to reproduce the issue:

Here is minimal repro: codesandbox.

Versions

Tested in MobX 6.

g6123 commented 3 months ago

The cause is simple. Theses checks are broken when called from another window:

https://github.com/mobxjs/mobx/blob/a73710cbe49c37fd5eff1665b9f2ae0883190baf/packages/mobx/src/utils/utils.ts#L143-L149

https://github.com/mobxjs/mobx/blob/a73710cbe49c37fd5eff1665b9f2ae0883190baf/packages/mobx/src/types/observablemap.ts#L353-L357

We can fix theses checks roughly like this:

export function isES6Map(thing: any): thing is Map<any, any> {
    return thing != null && thing[Symbol.toStringTag] == "Map";
    // or, `Object.prototype.toString.call` instead of `Symbol.toStringTag` for better browser coverage
}

export function isES6Set(thing: any): thing is Set<any> {
    return thing != null && thing[Symbol.toStringTag] == "Set";
}
            } else if (isES6Map(other)) {
                if (other.constructor.name !== "Map") {
                    die(19, other)
                }
                other.forEach((value, key) => this.set(key, value))

This S.O. answer might be helpful: https://stackoverflow.com/a/29926193

g6123 commented 3 months ago

I made a PR for the fix! #3893