Don't use deprecated lifecycle hook

[kevva]

When testing my app using React.StrictMode, it complains about the usage of this lifecycle hook. It also complains about the componentWillUpdate one, but it seems like it has to stay to avoid flashes of unstyled content.

[giuseppeg]

I think that we shouldn't use the constructor because that is not "async safe". Probably we should move the side effect to render.

@bvaughn I hate to bother you but I would love to know your opinion on this. Basically we are creating or updating styles and I am afraid that if we do so in componentDidMount and componentDidUpdate we'll get FOUC. Is it a terrible idea to move the side effect to render instead?

To give you a bit more context this component renders styles in the head and goes along other components:

<div>
  <_JSXStyle id={123} css="div { color: red } p { color: green }" />
  <p>howdy</p>
</div>

[bvaughn]

Hi!

The constructor, componentWillUpdate, and render are all equally unsafe for side effects because they are all part of the "render" phase.

I don't have a lot of context for this problem, so please take my suggestion with a grain of salt, but I would move the styleSheetRegistry.add call to componentDidMount (where side effects are safe). The browser won't paint/render an update until React finishes invoking the "commit" phase lifecycles so users shouldn't actually see a FOUC (unless styleSheetRegistry.add is somehow async). Any setState calls during this phase are also processed synchronously (i.e. before the browser paints) which is something libraries like react-virtualized rely on for measuring and positioning dynamic content.

This approach won't work for server-side rendering, since nothing "mounts" on the server. If you need to be able to support that as well, you could also leave the code in the constructor and run it only when typeof window === 'undefined'. You might be interested in RFC reactjs/rfcs/pull/8 for a possible better long-term solution.

[giuseppeg]

@bvaughn 👋 thanks for the clarification! I think you suggested this a while ago but back then I didn't think about moving the side effect to render.

and render are all equally unsafe for side effects

Fair enough! We'll give didMount/Update a try.

@kevva do you have an app to test this out?

Or (better/optional) if you have time add an e2e test? I once added this https://github.com/zeit/styled-jsx/blob/master/test/browser.js but jsdom is not good for these kind of things. You could replace that with tape and tape-run or puppeteer. Here's an example with tape https://github.com/giuseppeg/suitcss-toolkit/tree/master/packages/components-dropdown

[giuseppeg]

@kevva can you move styleSheetRegistry.add to componentDidMount and styleSheetRegistry.update to componentDidUpdate? Or do you want me to take over this PR?

[giuseppeg]

Fixed in #464