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

MobX 5.5 + React Native + @computed RangeError: Maximum call stack size exceeded #1777

Closed
creativefctr opened this issue Oct 13, 2018 · 35 comments

Comments

@creativefctr
Copy link

creativefctr commented Oct 13, 2018

Hi,
I got react native and typescript running, updated the JSC (altered gradle build config) to the latest version to support MobX 5 (installed via npm). Everything works fine except for @computed

@observer
export default class App extends Component<Props> {
  @observable public countVal:number = 0;
  constructor(props, context){
    super(props, context);
      this.onPressLearnMore = this.onPressLearnMore.bind(this);
      this.countVal = 0;
  }
  componentDidMount(){
      console.log('started!');
  }
 onPressLearnMore(){
      this.countVal = this.countVal + 1;
      console.log(this.countVal);
  }
  @computed get twoX(){
      return this.countVal * 2;
  }
  render() {
      console.log('rendered!');
      return (
      <View style={styles3.container}>
        <Text style={styles3.welcome}>Welcome to react!</Text>
        <Text className={styles2.blue}>Welcome {this.countVal} & {this.twoX}</Text>
        <Text style={styles3.instructions}>{instructions}</Text>
          <Button
              onPress={this.onPressLearnMore}
              title="Learn More"
              color="#841584"
              accessibilityLabel="Learn more about this purple button"
          />
      </View>
    );
  }
}

This produces the error:

RangeError: Maximum call stack size exceeded

The stack trace is full of "get"calls in mobx.module.js:295:20 , so it means get is being run recursively.

  • Downgrading to MobX v4.4 fixes the issue

Thanks

@creativefctr creativefctr changed the title MobX 5 + React Native + @computed RangeError: Maximum call stack size exceeded MobX 5.5 + React Native + @computed RangeError: Maximum call stack size exceeded Oct 13, 2018
@mweststrate
Copy link
Member

mweststrate commented Oct 13, 2018 via email

@creativefctr
Copy link
Author

I checked all of them. I did not find any solution provided, only one by yourself if I am not mistaken that was about modifying babel.rc file which did not work and had no effect.

@mweststrate
Copy link
Member

mweststrate commented Oct 13, 2018 via email

@creativefctr
Copy link
Author

I don't think so! RN works with 4.4 (with old or updated JSC) anyway, 5.5 with the newly updated JSC has issues and 5.5 does not work on the normal react native at all. And even if it's RN, I think if you create an issue for them it would be much more sensible to get this over with.

We are talking about a very large audience base of MobX being left out in a major version upgrade ... I think we deserve more than a workaround, I believe v5.5 must be usable by everybody in the normal way so I would appreciate if you dig into this problem a little bit more.

Right now, if someone runs npm install mobx + npm install mobx-react, version 5 will be installed and they will get errors that Proxy and Symbol are not supported.
Many people, like me, would try to fix that by updating JSC instead of downgrading mobx. I mean I prefer the latest version of a library to its old one because the new version is being actively maintained and then everything works fine except for @computed and it's a bit disappointing to downgrade a major version just because of that!

@creativefctr
Copy link
Author

and by the way, the reason I fell for MobX was the decorator syntax and I am very uncomfortable ditching that!

@mweststrate
Copy link
Member

mweststrate commented Oct 13, 2018 via email

@creativefctr
Copy link
Author

OK, then it's your call... but if you can find a fix it would be much better.
Close this issue if you plan on no fix.
Tnx

@mweststrate
Copy link
Member

@xfactor5 could you answer the following questios for some deeper investigation?

  • This problem only applies only to RN 57 and higher, with upgraded JSCore on android, correct?
  • Does the problem also occur when @computed is not used on React component, but on a 'normal' class?
  • Does this problem occur if decorate utility is used rather than the decorators syntax (decorate(Class, { prop: computed })?

@creativefctr
Copy link
Author

Sure...

  1. Yes but I have not checked 5.0 till 5.7 looks like it's a 5.0+ issue (and v5 does not work at all in normal JSC)
  2. No, I did not check all of the MobX features but normal observables were working just fine. The problem only happens when @computed is added.
  3. I checked and the issue seems to happen with decorate syntax too, same callstack error!

@Strate
Copy link
Contributor

Strate commented Oct 18, 2018

I have almost same problem with RN0.56

@mweststrate
Copy link
Member

mweststrate commented Oct 18, 2018 via email

@Strate
Copy link
Contributor

Strate commented Oct 18, 2018

@mweststrate this is clean app for RN0.56: https://github.com/ncuillery/rn-diff/tree/rn-0.56.0/RnDiffApp
and this is for 0.57.2: https://github.com/ncuillery/rn-diff/tree/rn-0.57.2/RnDiffApp

You can check versions of babel, .babelrc files and so on

@mweststrate
Copy link
Member

I tried reproducing the issue, but I am a total RN / android noob, and I have no clue what upgrading gradle files means :-). Could some setup an expo based project which I can just star, scan the code an be on my way?

@mweststrate
Copy link
Member

@xfactor5 also, for clarification "The problem only happens when @computed is added" with added, you mean to a react component, correct? @computed on normal classes (e.g. a store) works fine?

@mweststrate
Copy link
Member

In the mean time, updated the Readme to reflect that for now probably it is the most convenient to just stick to MobX 4 for React Native Android. I doubt upgrading JSC core, for the sole purpose of being able to use MobX 5, is justifiable at this point.

@creativefctr
Copy link
Author

React component is for sure but I have not tested on others. Last time I upgraded to MobX 5 to test for you, I encountered a react native bug and got stuck for 2 hours so, to be honest, I am not gonna test that again :D If I get time I will check it and report back.
RN is a real headache!
Thanks

@mweststrate
Copy link
Member

N.B.: This solution is known to work for some at least, so just linking it here again for any future readers :). oblador/react-native-vector-icons#801 (comment)

@mweststrate
Copy link
Member

mweststrate commented Oct 20, 2018

Another observation: seems at least 25 people were able to get MobX 5 + RN Android working properly if I read these results correctly. Curious where the differences are. Minimal reproduction would be greatly appreciated. https://twitter.com/mweststrate/status/1053265411834372096

@creativefctr
Copy link
Author

I don't know maybe they did not use computed ... or maybe some different config or plugin set in babel or even maybe no typescript?

@kerwintang
Copy link

I also encountered this and workarounds don't work. Are there any plans on having this resolved on mobx side?

@rvion
Copy link

rvion commented Nov 27, 2018

did not read how other people fixed it, but I fixed this issue by removing circular dependencies.

tip: use madge --circular . to find the culprit

refs: https://www.npmjs.com/package/madge

@rvion
Copy link

rvion commented Nov 27, 2018

this solution worked on 3/4 differents packages showing the problem.

@psycura
Copy link

psycura commented Nov 28, 2018

@rvion But how exactly it work?

@rvion
Copy link

rvion commented Nov 28, 2018

Don't know, I didn't spend time trying to understand. I guess it depends what your transpiler / runtime module system is. On a project with typescript (w/ decorator + decorator metadata) and parcel bundler, for instance problem only occured during module hot reload when decorate was run.

@mweststrate
Copy link
Member

Closing for now, clearly, it is not solved for everyone, but it isn't failing for everyone either. So there are external factors in play.

However, without a reproducible setup this can't be investigated.

Please refrain from commenting with additional examples of this problem without reproducible setup.

@kirushi
Copy link

kirushi commented Dec 2, 2018

Running into this issue for [email protected] using [email protected] & [email protected].

If I use a computed value on my decorated class I get the following error: RangeError: Maximum call stack size exceeded.

Everything else is working fine. It's a shame that we can't utilise the computed feature for React Native!

@mweststrate to reproduce - create a fresh react native app:

  1. Create a basic MST with some dummy data. Pass it in as props to your <App store={store} /> component from your index file.
  2. Add the observer decorator to your App class.
  3. Add @observable value = 1 to your App class.
  4. Add @computed get sum() { return this.value + 2 }
  5. Create a function to update your this.value.
  6. Run app and view call stack error.

@psycura
Copy link

psycura commented Dec 3, 2018

I found out, that this problem exists only when using @computed inside the react class component.
If i use this in the Store - everything works just fine

@mweststrate
Copy link
Member

mweststrate commented Dec 3, 2018 via email

@psycura
Copy link

psycura commented Dec 3, 2018

Yes, this can be achieved through dev -menu, but it doesnt help

@jakst
Copy link

jakst commented Dec 3, 2018

Here is a minimal project reproducing the issue:
https://github.com/Jakst/break-mobx5-computed-react-native
https://expo.io/@jakst/expomobxcomputed

@mweststrate mweststrate reopened this Dec 3, 2018
@mweststrate
Copy link
Member

Thanks for the repo @jakst!

@mweststrate
Copy link
Member

Fixed and released as 5.7.0

@Dakkers
Copy link

Dakkers commented Feb 4, 2019

@mweststrate there's a little blurb in the README that refers to this issue:

MobX >=5 runs on any browser with ES6 proxy support. It will throw an error on startup on older environments such as IE11, Node.js <6 or React Native Android on old JavaScriptCore how-to-upgrade. Warning: since upgrading JSC is non-trivial, and decorators can be troublesome as well in React Native, for now it is recommended to stick to MobX 4.x for for React Native Android development.

Is the part "decorators can be troublesome" still applicable? (It refers to this issue.)

@mweststrate
Copy link
Member

mweststrate commented Feb 4, 2019 via email

@lock
Copy link

lock bot commented Jul 21, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs or questions.

@lock lock bot locked as resolved and limited conversation to collaborators Jul 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants