mobxjs / mobx-react-lite

Lightweight React bindings for MobX based on React 16.8 and Hooks
https://mobx.js.org/react-integration.html
MIT License
2.13k stars 91 forks source link

Attempted import error: 'makeObservable' is not exported from 'mobx'. #319

Closed roygold7 closed 4 years ago

roygold7 commented 4 years ago

Intended outcome

I'm trying to import Observer and useObserver to my App.tsx in a create-react-app project like so: import { Observer, useObserver } from "mobx-react-lite";

Actual outcome

When importing Observer or useObserver from mobx-react-lite, an error is thrown because the library is trying to import makeObservable from mobx in the file assertEnvironment.ts. The problem is that it is not exported from mobx.

When adding this: import { Observer, useObserver } from "mobx-react-lite"; to a file, the following error is thrown:

./node_modules/mobx-react-lite/es/utils/assertEnvironment.js
Attempted import error: 'makeObservable' is not exported from 'mobx'.

Then when I look in ./node_modules/mobx-react-lite/es/utils/assertEnvironment.js, I see this:

import { makeObservable } from "mobx";
import { useState } from "react";
if (!useState) {
    throw new Error("mobx-react-lite requires React with Hooks support");
}
if (!makeObservable) {
    throw new Error("mobx-react-lite@3 requires mobx at least version 6 to be available");
}

How to reproduce the issue

Add import { Observer, useObserver } from "mobx-react-lite"; to the the App.tsx in a create-react-app project.

The only solution I found was to comment out parts of assertEnvironment.js:

// import { makeObservable } from "mobx";
import { useState } from "react";
if (!useState) {
    throw new Error("mobx-react-lite requires React with Hooks support");
}
// if (!makeObservable) {
//     throw new Error("mobx-react-lite@3 requires mobx at least version 6 to be available");
// }

Versions

"mobx": "5.15.7",
"mobx-react": "6.3.0",
"mobx-react-lite": "3.0.0"
danielkcz commented 4 years ago

Please either use mobx 6.x or mobx-react-lite 2.x. Your combination is incompatible.

roygold7 commented 4 years ago

Please either use mobx 6.x or mobx-react-lite 2.x. Your combination is incompatible.

Okay but the error needs to be thrown then, instead of the compiler just saying that the import can't be found. The code never reaches this part:

if (!makeObservable) {
     throw new Error("mobx-react-lite@3 requires mobx at least version 6 to be available");
 }

which is what should happen.

danielkcz commented 4 years ago

You are right it's not ideal. In theory, we could use dynamic import() instead. It gets transformed to require in CJS, so it should be safe. The only problem that in ESM (like React Native) it will check asynchronously and end the error cannot be thrown to kill your app. We would be only able to produce a warning.

Accepting PR.

mweststrate commented 4 years ago

You get an error at build time, rather than at runtime. That sounds like a benefit to me, not a downside. Although the error itself is more vague, googling it will now show up this issue so people should still be able to figure it out :).

On Thu, Oct 1, 2020 at 2:28 PM Daniel K. notifications@github.com wrote:

Reopened #319 https://github.com/mobxjs/mobx-react-lite/issues/319.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mobxjs/mobx-react-lite/issues/319#event-3829485759, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBAW7COLZKZNVKI5XXLSIR7WHANCNFSM4SAL5OSA .

mweststrate commented 4 years ago

Closing for now, this is supposed to fail. So I don't think there is much point in complicating stuff by making it fail differently.

mweststrate commented 4 years ago

That has been released 2 weeks ago already

On Thu, Oct 8, 2020 at 3:58 AM CDoughty08 notifications@github.com wrote:

@mweststrate https://github.com/mweststrate Would it be reasonable to release a patched mobx-react i.e. 6.3.1 that does not list "mobx-react-lite": ">=2.2.0". to prevent installing an incompatible version.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mobxjs/mobx-react-lite/issues/319#issuecomment-705300127, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBHD76MCLKAOMZZMEHDSJUTFHANCNFSM4SAL5OSA .