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

don't use instanceof operator #571

Closed
amir-arad opened this issue Sep 22, 2016 · 1 comment
Closed

don't use instanceof operator #571

amir-arad opened this issue Sep 22, 2016 · 1 comment
Labels

Comments

@amir-arad
Copy link

amir-arad commented Sep 22, 2016

In my experience there's a bug when you have more than one copy of Mobx in your module system.
The getAtom function checks for atoms using instanceof javascript operator, which uses nominal typings (equiv. to thing.__proto__.constructor === BaseAtom) instead of using structural / duck typing (typeof thing.reportObserved === 'function' && typeof thing.reportChanged === 'function').

so an atom created by mobx A is not recognised by mobx B.

The rationale here is, when you have multiple instances of the Mobx module, you have multiple instances of the module's functions, with no pointer equality between the two otherwise matching functions. Effectively makes the constructors themselves part of a module's state (something we wish to avoid, judging by the global singleton GlobalState).
This can explain the error message in #570 when using npm 2 or some other modules setup that did not go through aggressive tree-shaking.

@amir-arad amir-arad changed the title don't use instanceof operator in modules don't use instanceof operator Sep 22, 2016
@amir-arad
Copy link
Author

I'd implement a type guard for that type check, It improves code readability and is well suited for runtime contract checks.

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

No branches or pull requests

2 participants