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

[issue] "e.observing is null" since last update of firefox #614

Closed
AoDev opened this issue Oct 17, 2016 · 25 comments
Closed

[issue] "e.observing is null" since last update of firefox #614

AoDev opened this issue Oct 17, 2016 · 25 comments

Comments

@AoDev
Copy link

AoDev commented Oct 17, 2016

Hello, our app started to have an issue on Firefox with their last update:

e.observing is null

with mobx: 2.5.2, mobx-react: 3.5.6,

(for example I had FF 47.8, I think, and it updated to 49.0.1)

Since then we have an issue related to a <select /> which values are observables.
When a value is selected, it throws e.observing is null.

This bug was confirmed by various people, developers and customers.

@mattruby
Copy link
Member

Can you please create a fiddle recreating this issue.

On Oct 17, 2016 3:33 AM, "AoDev" [email protected] wrote:

Hello, our app started to have an issue on Firefox with their last update:

e.observing is null

with mobx: 2.5.2, mobx-react: 3.5.6,

(for example I had FF 47.8, I think, and it updated to 49.0.1)

Since then we have an issue related to a which values are observables. When a value is selected, it throws e.observing is null. This bug was confirmed by various people, developers and customers. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com//issues/614, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIrcqGQ-coXzEbpjFKv89oxG4Iocbfxks5q0zL1gaJpZM4KYW-m .

@AoDev
Copy link
Author

AoDev commented Oct 17, 2016

I made one but I can't reproduce this issue with a simple test case. Right now we have seen that it fails in production while working in our staging. I am still investigating. Maybe the issue is on our end only finally.
I'll try to give more feedback and see if it's really related to mobx. Do you have any hint regarding this specific message? when why it can happen?

@AoDev
Copy link
Author

AoDev commented Oct 17, 2016

Found update related to selects in Firefox (from 47 to 48)

https://fossies.org/diffs/firefox/47.0.1.source_vs_48.0.source/toolkit/modules/SelectContentHelper.jsm-diff.html

@mweststrate
Copy link
Member

I would expect all data objects to be untouched by the DOM, but to avoid
any issues, you could pass plain objects to the DOM, and maintain a simple
mapping which is used in your event handlers to find the real object back?

Op ma 17 okt. 2016 om 16:59 schreef AoDev [email protected]:

Found update related to selects in Firefox (from 47 to 48)

https://fossies.org/diffs/firefox/47.0.1.source_vs_48.0.source/toolkit/modules/SelectContentHelper.jsm-diff.html


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#614 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABvGhGKLslk0NOD7s-UZkSb2Iof4ZQBpks5q0408gaJpZM4KYW-m
.

@AoDev
Copy link
Author

AoDev commented Oct 18, 2016

Here is a codepen which is as similar as our original code as possible:
http://codepen.io/AoDev/pen/KgxGWm?editors=0010#0

About the code
What is fed to the select itself is an array resulting of a computed value. What I am not sure in mobx is when a computed value is an array or an object, are their properties/elements observables themselves? For what I've seen, computed values work more like mobx.asReference in that case, so the items are plain JS objects, am I wrong?

About the codepen
Unfortunately it does not trigger the error but still wanted to share it. To be honest, the fact that Mozilla made a change to the select element in the previous version and our select component breaking at the same time, only in Firefox (worked fine in v47) does not seem like a coincidence. But so far I can't reproduce it easily.

Maybe by looking at the codepen and their changes you can see something.

Especially these changes of theirs are "suspicious":

        this.global.addEventListener("mozhidedropdown", this);
        let MutationObserver = this.element.ownerDocument.defaultView.MutationObserv
    er;
        this.mut = new MutationObserver(mutations => {
          // Something changed the <select> while it was open, so
          // we'll poke a DeferredTask to update the parent sometime
          // in the very near future.
          this._updateTimer.arm();
        });
        this.mut.observe(this.element, {childList: true, subtree: true});

You can see all their changes in the link I've added in a previous comment.
I'll keep you posted with our findings.

@kcampbell
Copy link

kcampbell commented Oct 31, 2016

We were seeing a very similar issue in our app: javascript error with the message TypeError: observing is null. This was happening in Firefox 49 but not in Firefox 46 (I didn't test versions in between). Also not happening in Chrome.

It was only reproducible on production environments (which use bundled/transpiled javascript from webpack and babel), so hard to pinpoint exactly where the error was getting thrown.

The solution I found was to remove the @observer decorators from several low-level React components that were not accessing any @observable fields. Refactoring the same components to actually use observables also seemed to fix the error.

@AoDev
Copy link
Author

AoDev commented Nov 4, 2016

Like @kcampbell I have decided to remove the observer capabilities for the components that holds the select. This has "solved" the issue. Just that we lose the mobx optimization with shouldComponentUpdate. The "bug" itself I could not investigate or find anything really useful.

@mweststrate
Copy link
Member

Closing the issue for now

@mcav
Copy link

mcav commented Dec 2, 2016

I ran into this too under similar conditions (an @observer component that doesn't listen to any observables, but has child @observer components that do). While I haven't found a reproducible small test case, I do have a repo that exhibits the issue. It seems like we're hitting a Firefox JS engine bug, as unlikely as that sounds, but I haven't found a matching bug in Bugzilla. That would corroborate the reports that this only happens in certain environments (prod vs. dev, maybe related to the level of optimizations kicking in?).

In bindDependencies, under certain conditions, the assignment derivation.newObserving = null appears to be taking effect before the assignment of observing and derivation.observing. In this screenshot, note the values of window.a (displayed in the console) and b (displayed in the sidebar): a is an Array[100] as expected, but b is null when it should be the same object as a.

Moving the derivation.newObserving = null assignment later in the function (e.g. after the first loop) seems to mitigate the problem, as do various other benign tweaks to the function. Unfortunately, if the cause is truly a Firefox JS engine bug, that would only be a bandaid. This doesn't seem to be a problem with mobx itself.

I was able to repro with the following:

  1. Clone https://github.com/mozilla-sensorweb/sensorweb-website/tree/mobx (note the mobx branch)
  2. install, ideally just npm install && npm run build
  3. open build/index.html
  4. click a couple times on the "month" button in the sidebar

disclaimer: I work for Mozilla but not on the Firefox team.

@jmca
Copy link

jmca commented Dec 21, 2016

FYI, I was also having this issue the other day with FireFox. I updated mobx & mobx-react to the latest versions and it has resolved the issue thus far.

@AoDev
Copy link
Author

AoDev commented Dec 21, 2016

@jmca Hi, just for any person reading that later, could you tell us which version was the latest when you updated?

@jmca
Copy link

jmca commented Dec 22, 2016

@AoDev Here's my upgrade path:
mobx: 2.6.0 --> 2.7.0
mobx-react: 3.5.7 --> 4.0.3

@jukben
Copy link

jukben commented Jan 4, 2017

@jmca Thanks, this solve the observing is null issue for me too.

@drewhamlett
Copy link

drewhamlett commented Jan 10, 2017

I had latest versions and still was happening. I had a component like this.

@observer
class UndoButton extends React.Component {
  onUndo = () => {
    this.props.bulkUploadItem.undo()
  }

  render() {
    return (
      <button className="button warning small" onClick={this.onUndo}>
        Undo
      </button>
    )
  }
}

I took @observer off and everything started working

@mcav
Copy link

mcav commented Jan 10, 2017

@drewhamlett This happens only in Firefox, right? Do you have any publicly-available code that can reliably reproduce the issue?

@drewhamlett
Copy link

I can try to whip something up tonight possibly.

@dennyferra
Copy link

dennyferra commented Apr 7, 2017

Had a similar issue. Older version of Firefox worked fine, latest (51.0.1) would fail with observing is null error. Seems to be similar to what @drewhamlett showed above.

Our problem was that we had a component with the @observer attribute but the component had nothing to observe:

import React, { Component } from 'react';
import { observer } from 'mobx-react';

@observer
export default class ContentFooter extends Component {
  render() {
    return (
      <div className="footer" />
    );
  }
}

Removed the @observer attribute and it worked.

"mobx": "^2.7.0"
"mobx-react": "^4.0.4"

@mtyaka
Copy link
Contributor

mtyaka commented Sep 18, 2017

@mcav I came to the same conclusion - this is a bug in Firefox JS engine. Turning javascript.options.baselinejit off in about:config "fixes" the problem. Did you ever open a bug in bugzilla?

The problem is still present in FF 55.

@mcav
Copy link

mcav commented Sep 18, 2017

I've filed a bug on Bugzilla here (today) with my best description for what might be going on. It's unfortunate that we haven't found a nice small test case that doesn't require a viewer to dive into MobX internals. Hopefully someone who works on the JIT might know how to trigger it deterministically on a small test case.

@mtyaka
Copy link
Contributor

mtyaka commented Sep 19, 2017

Thanks @mcav!

@mweststrate
Copy link
Member

mweststrate commented Sep 19, 2017 via email

@mweststrate
Copy link
Member

@mcav

if in your local modules, you rewrite the double assignments as

var observing = derivation.newObserving;
derivation.observing = observing;

Does that remove the problem? That would be a simple work around for now

@mtyaka
Copy link
Contributor

mtyaka commented Sep 19, 2017

@mweststrate That did not fix the problem for me, but moving the derivation.newObserving = null assignment below this loop seems to work.

@mweststrate
Copy link
Member

@mtyaka / @mcav Just moved that assignment down in the function body, could you verify whether that workaround fixes the issue in [email protected]?

@mweststrate
Copy link
Member

Probably fixed by work-around in 3.3.0 and further depends on the release of the linked issue in Firefox. Closing it here

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

No branches or pull requests

10 participants