react-pdf v2 and react-virtualized

[michaeldzjap]

I am trying to use the new version of this lib together with react-virtualized. As far as I can test so far I have managed to get this working with a fully responsive pdf viewer. The only problem is that I receive multiple (depending on the number of pages of the pdf) React warnings of the form

warning.js:36 Warning: setState(...): Can only update a mounted or mounting component. This usually means you called setState() on an unmounted component. This is a no-op. Please check the code for the Page component.

I have tried to track this down myself and although I am not sure about this at all, it seems it might be related to the fact that I use the CellMeasurer component from react-virtualized. From the react-virtualized documentation:

High-order component that automatically measures a cell's contents by temporarily rendering it in a way that is not visible to the user.

Not very clear how this is done, but I suspect that maybe this rendering is happening very fast and perhaps

componentWillUnmount() {
    if (this.runningTask && this.runningTask.cancel) {
      this.runningTask.cancel();
    }
  }

is not cancelling "quick" enough (i.e. setState({}) in onLoadSuccess()/onLoadError() is called even though Page has unmounted already). But at this point this is more speculation than something that I have been able to accurately verify. I also understand that this is not really a react-pdf issue per se, but nonetheless I was wondering if there perhaps is anyone who could shed some light on this matter.

[wojtekmaj]

Hey,
thanks for the report. Could you also include stack trace of this warning?
On the side note, it's a very bad idea to temporarily render these components. Not only they can resize themselves on the fly when the page is loaded completely, but also it's very resource consuming. PDF.js API included the way to get page height in different ways.

[michaeldzjap]

Here's the full stack trace:

Yeah, I agree. Seems like quite a rough way to go about things. But that is how CellMeasurer works unfortunately. If my viewer was not fully responsive I could probably ditch that component and just set row heights using onLoadSuccess callback of Page. But now I have to make sure the react-virtualized row heights are re-computed appropriately as the window (and consequently the page containers) is resized. Otherwise the virtualization process of react-virtualized seems to become one big mess.

[wojtekmaj]

Thanks for all the information! :)
Please check out version 2.0.0-beta.4. I believe I've fixed your problem. Please let me know!

PS. I'm waiting for your feedback and I'm releasing 2.0.0, enough of that beta stage ;)

[michaeldzjap]

No, unfortunately that did not help. Looking at the stack trace again, it seems like it might be the setState({}) in onLoadSuccess() that is causing those print warnings, although at no point in the stack trace I actually see componentWillUnmount() being called on Page...

I can make a little sample project with react-virtualized + react-pdf if that would make it easier for you. Let me know. And thanks for all your work on this.

[wojtekmaj]

Aw, shoot. We're getting somewhere though as I see that onPageError does not throw a warning anymore!
Yes, that would be lovely to have such a sample project, I'd do my best to fix this.
Thank you!

[michaeldzjap]

Here's a little sample project. Install it with:

• npm install
• npm run build (or npm run watch)

I've tested it on latest Chrome together with http-server to serve my pdf over http. You should see the setState({}) warnings in the console. Interestingly enough, testing the project on Firefox doesn't produce any setState({}) related warnings!

I also noticed react-virtualized performance on Firefox is pretty poor (pdf is flickering on scroll). I was kind of baffled by this at first, as my original project seems to do just fine on Firefox, but then I remembered I am using my fork of react-pdf for my original project where I have added a small optimisation to Page (plus the ability to pass children to Page):

shouldComponentUpdate(nextProps, nextState) {
    return nextState.page !== this.state.page;
}

Thanks!

react-pdf-sample.zip

[wojtekmaj]

Hey @michaeldzjap, I was playing for several hours with the demo you've sent me but I cannot find the root of this issue. Did some small fixes for the next release but I doubt they will help. Somehow cancelled promises are still called.

I could do some hacks (actually, pretty simple ones) to get it working but I'd really want it to be done properly and I can't pin the issue down.

Whatever renders those components is behaving super crazy... Can't even find a deterministic logic behind it. The components throwing errors seem to change every so often.

[michaeldzjap]

Thanks @wojtekmaj. I wanted to take a crack at this myself the other week, but ended up being too busy with other work.

I did just now try one thing, which seemed to do the trick actually! I got the idea from reading this. Basically, I have introduced an this._isMounted property for Page which I set to true in Page.componentDidMount() and to false in Page.componentWillUnmount(). Then I changed Page.onLoadSuccess() to:

onLoadSuccess = (page) => {
    if (this._isMounted) {
      this.setState({ page });

      const { scale } = this;

      callIfDefined(
        this.props.onLoadSuccess,
        {
          ...page,
          // Legacy callback params
          get width() { return page.view[2] * scale; },
          get height() { return page.view[3] * scale; },
          scale,
          get originalWidth() { return page.view[2]; },
          get originalHeight() { return page.view[3]; },
        },
    );
 }

I think using an _isMounted variable is considered an anti-pattern though, so this solution might be a bit hacky. But it does get rid of the setState({}) related warnings in the console.

I do realise that my use case might be a bit specific and perhaps you don't want to mess up your code with a somewhat hacky solution. Although you could also argue that using a virtualization lib together with react-pdf is the saner choice when you want to display pdf's with a lot of pages. What do you think?

By the way; I made a repository for a sample project that uses react-pdf + react-virtualized. Its a bit cleaner than the .zip I included previously here.

[michaeldzjap]

I have investigated this issue a bit more. It is not an explanation for how to fix this more cleanly (i.e. not using an _isMounted variable), but I have found a way to work around it for those who wish to use react-pdf together with react-virtualized and not run into this problem.

I won't go into too much detail here about react-virtualized, but the main culprit of the issue turned out to be the CellMeasurer component. I was using this component to dynamically recalculate the height of my rows on a resize action (to create a fully responsive viewer) and account for pdf documents that possibly have pages with different widths and/or heights. I replaced the original CellMeasurer based solution for a custom page height cache solution where I compute the row/page heights manually. Not only did it solve this problem, but it also solved a bunch of other scrolling related issues I had before with react-virtualized.

I have updated the sample project repository to reflect the changes for those who are interested.

So in conclusion, although I haven't found a real solution for this issue, I have found a way to make react-pdf and react-virtualized play nice together. Since this was my original objective this issue can be closed as far as I am concerned.