errors in widgets.js with react 15.4

[openciti]

The timeline renders but I'm getting Uncaught errors in the console

Uncaught (in promise) TypeError: Cannot read property 'classList' of null
at Object.u [as present] (widgets.js:8)
at t.hasRootClass (widgets.js:9)
at t.recalculateBreakpoints (widgets.js:10)
at t. (widgets.js:10)
at widgets.js:9

Uncaught (in promise) Error: sandbox not initialized
at t.removeRootClass (widgets.js:9)
at t.recalculateBreakpoints (widgets.js:10)
at t. (widgets.js:10)
at widgets.js:9

[andrewsuzuki]

Looks like this is due to a recent change in the remote-loaded widgets library, as evidenced by this recent post on the twitter forums.

The problem here is that React will remove the widget element, but the widget library isn't notified (and doesn't have any method of notification, as there is no destroy method).

I'm not really sure if there's a solution here.

[openciti]

thanks.

[epitaphmike]

I am running a Follow button and a Timeline. It seems fine with only the follow button. The Timeline is the issue in my case. I posted another thread in hopes for a response. It's not very helpful that they automatically close threads after 14 days.

https://twittercommunity.com/t/timeline-widget-not-destroyed-on-single-page-application-page-change-spa/84023

[epitaphmike]

I received a response from twitter. I am going to whip up a test page for them to look at. I'll report back any details soon.

[epitaphmike]

I created a repo with demo to test the issue we've been having and surprisingly did not get any errors.

https://github.com/epitaphmike/react-twitter-bug

The only main difference is I used react-router v.4.0.0 instead of react-router v3.0.0 and the fact that the demo is not our entire app. I am going to make a branch and use react-router v3.0.0 to see if I can reproduce.

[andrewsuzuki]

Hmm, interesting! Are you using react-router 4 in your app as well?

[epitaphmike]

No. We are using react-router v3.0.0 currently. It is in the cards to upgrade, I just haven't yet.

[epitaphmike]

I just updated the repo with a branch using react-router-3.0.0 and still no bug. I'll have to do more digging to see what is causing this.

[pjjanak]

I am working on the project with @epitaphmike and figured out what our actual issue was. Basically any time the component which contains any component from this library re-renders, the Twitter component also re-renders. I imagine that rather than dumping the widget, if all other props are the same, and trying to re-render it, it might be a better idea to just return false from componentShouldUpdate in AbstractWidget in almost all cases. I think that would effectively solve the issues we're seeing.

[andrewsuzuki]

Hmm, shouldComponentUpdate is already implemented in the individual widget react components (Timeline, Follow, etc). It does a deep equality check for its props. In the case of Timeline, it checks dataSource and options.

[pjjanak]

@andrewsuzuki looks like default values get added to the dataSource props object if they aren't specified by the caller. The nextProps doesn't have them. I think the Twitter call is probably the thing adding them.

Specifically in our case this.props.dataSource has a sourceType and screenName specified and keys for lang, tweetLimit, and showReplies all undefined. nextProps.dataSource only has sourceType and screenName. So lodash's equals function is returning that the two are different, when they're functionally the same.

[andrewsuzuki]

Ah, that's it then...Twitter mutates the object instead of cloning.

So, three possible paths here:

1. We can try to get Twitter to do a simple Object.assign({}, ...) on incoming object arguments.

2. Don't check for changes to object properties in shouldComponentUpdate.

3. Always return false in shouldComponentUpdate; don't check for updates to props at all.

Thoughts?

[epitaphmike]

Twitter may be open to making a change. I can reach out in the open thread I currently have with them in the developer forum. In the meantime, I'd always return false from shouldComponentUpdate as the main changes are happening from Twitter outside the React lifecycle. If a user of this library needs to do something like change the screenName or something along those lines, they can adjust the shouldComponentUpdate accordingly.

[pjjanak]

As far as options 2 and 3 go, changing the source data of the widget should probably cause it to update to reflect that, however I feel like it will then just run into this error anyway. Obviously option 1 would be better for Twitter to do from a best practices standpoint, but I'm wondering if it matters if/until they implement some sort of destroy call you can make against their api?

[andrewsuzuki]

@epitaphmike Sounds good, let's see what they say.

For reference, here's a temporary fix:

// Timeline.js

import React from 'react'
import { Timeline } from 'react-twitter-widgets'

export default class Timeline extends React.Component {
    shouldComponentUpdate() {
        return false
    }

    render() {
        <Timeline {...this.props} />
    }
}

[andrewsuzuki]

@pjjanak You're right, the error will likely show up regardless. Really the only solution here is getting them to implement a destroy method.

[epitaphmike]

Good News @andrewsuzuki Twitter filed a bug internally to address the Object.assign({}, ...).
I am not sure about the destroy method. I asked for more details.

[epitaphmike]

Looks to be something else other than the above code. I was able to get it to render no problem.

[efosao]

Simplifying @andrewsuzuki's temporary-fix by using PureComponent instead of shouldComponentUpdate() also works.

import React, { PureComponent } from 'react'
import { Timeline } from 'react-twitter-widgets'

export default class TimelineFix extends PureComponent {
  render () {
    return <Timeline {...this.props} />
  }
}