ManifoldScholar / manifold

Transforming scholarly publications into living digital works.
http://manifoldapp.org
GNU General Public License v3.0
236 stars 31 forks source link

Update errors on several components #925

Closed SMaxOwok closed 6 years ago

SMaxOwok commented 6 years ago

Can only update a mounted or mounting component. This usually means you called setState, replaceState, or forceUpdate on an unmounted component. This is a no-op.

Event.Event ResourceListSlideCaption

SMaxOwok commented 6 years ago

These are being caused by dynamic requires

componentDidMount() {
  import(/* webpackChunkName: "autolinker" */ "autolinker").then(
    autolinker => {
      this.setState({ autolinker });
    }
  );
}
SMaxOwok commented 6 years ago

@zdavis it seems like importing these modules as we do normally resolves the warnings and preserves functionality.

Is there a reason why the above pattern is being used over something like import { autolinker } from "autolinker";?

zdavis commented 6 years ago

Yes. We're using code splitting to avoid pulling in libraries until they're needed. It's not a big deal with this one, but with things like the code editor, it makes a huge difference in our bundle size.

I can tackle this as part of a larger attempt to split apart the bundle.

SMaxOwok commented 6 years ago

@zdavis how do you feel about https://github.com/jamiebuilds/react-loadable?

It seems like our issues are resolved doing something like below. I'm seeing the chunk loaded with the page and we don't get any warnings. This is a quick change, so I'll open a PR for these usages. Feel free to close it if you want to go a different way.

import Loadable from "react-loadable";

const Velocity = Loadable({
  loader: () =>
    import(/* webpackChunkName: "velocity-react" */ "velocity-react").then(
      Velocity => Velocity.VelocityComponent
    ),
  loading: () => null
});

class ResourceListSlideCaption {

  render() {
    <Velocity {...animation}>
      {resourceSlideCaptionStuff}
    </Velocity>
  }
}
zdavis commented 6 years ago

This approach works for me. Thanks!