Skip to content

React 16.3 deprecated componentWillMount and SSR? #12495

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

Closed
budarin opened this issue Mar 30, 2018 · 34 comments
Closed

React 16.3 deprecated componentWillMount and SSR? #12495

budarin opened this issue Mar 30, 2018 · 34 comments

Comments

@budarin
Copy link

budarin commented Mar 30, 2018

Hi!

Help me understand how to do refactoring:
deprecated componentWillMount is recommended to replace with componentDidMount (this method is not called on the server-side) or with static getDerivedStateFromProps which has no access to a particular instance.

so where to put the code which must be running on both sides after initialization of an instance?

@TrySound
Copy link
Contributor

@budarin There is mention in Note in blog post
https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html#fetching-external-data

When supporting server rendering, it’s currently necessary to provide the data synchronously – componentWillMount was often used for this purpose but the constructor can be used as a replacement. The upcoming suspense APIs will make async data fetching cleanly possible for both client and server rendering.

@budarin
Copy link
Author

budarin commented Mar 30, 2018

@TrySound
I'm sorry - I don't need to fetch data )

Imagine component like a Loadable: after initialization it has to check for loading component and initialize its loading on both sides: browser and server.

Constructor is not suitable place for this - there is no access for the components instance.
And componentDidMount is not called on server side

@TrySound
Copy link
Contributor

But constructor has an access to instance. What do you mean?

Constructor is not suitable place for this - there is no access for the components instance.

@budarin
Copy link
Author

budarin commented Mar 30, 2018

@TrySound
thanks for pointing )
I'm sorry - I was mistaken !
I have a bug in my code...

@budarin budarin closed this as completed Mar 30, 2018
@oyeanuj
Copy link

oyeanuj commented Mar 31, 2018

@TrySound Given this change, should the docs be updated for the React API here where it essentially says not to have any side-effects in the constructor? Anything for developers to watch out for?

cc: @bvaughn

@gaearon
Copy link
Collaborator

gaearon commented Mar 31, 2018

Setting the initial state is not a side effect.

@oyeanuj
Copy link

oyeanuj commented Mar 31, 2018

@gaearon To clarify, I was suggesting clarifying the answer to the following question: _What lifecycle method should one use on the server to fetch data (say from Redux action or XHR), given that componentWillMount is going away and componentDidMount doesn't execute on the server?

@bvaughn
Copy link
Contributor

bvaughn commented Apr 1, 2018

We're still working on a good story for server-side data fetching (dubbed "suspense").

componentWillMount is going away, but UNSAFE_ componentWillMount is still here (and won't be going anywhere). There's also the constructor, which is the same (equally bad) as componentWillMount was.

@gaearon
Copy link
Collaborator

gaearon commented Apr 1, 2018

What lifecycle method should one use on the server to fetch data (say from Redux action or XHR), given that componentWillMount is going away and componentDidMount doesn't execute on the server?

To be extra clear componentWillMount was never very helpful on the server for data fetching since it executes synchronously.

So you had to render in two passes (once to start the fetch and then another time after it’s resolved). It’s not an optimal architecture but while you depend on it you can keep using UNSAFE_componentWillMount for the same purpose. (Or constructor, I guess.)

When “suspense” API is ready, it will be the preferred solution because it will let you wait for data both on the client and on the server. Without rendering twice.

@budarin
Copy link
Author

budarin commented Apr 1, 2018

@gaearon

I have a problem with new lifecycle
I have a HOC to add/remove styleSheets from components module using CSSModules in useable mode:

function withStyles(styles) {
        return function(BaseComponent) {
            class componentWithStyles extends React.Component {
                count: number = 0;

                componentWillMount() {
                    // We need a counter because with Fiber cWM may be called multiple times
                    this.count += 1;

                    if (this.count === 1) {
                         styles.use();
                    }
                }

                componentWillUnmount() {
                      styles.unuse();
                }

                render() {
                    return <BaseComponent {...this.props} css={styles} />;
                }
            }

            return componentWithStyles;
        };
    }

when I move code from componentWillMount to constructor I'v got a strange bug: styles doesn't work correctly as before.
I have different styles for a page layout component for different routes. When I move code to a new lifecycle - styles are not removed when I return from a route to previous

function withStyles(styles) {
        return function(BaseComponent) {
            class componentWithStyles extends React.Component {
                count: number = 0;

                constructor(props) {
                    super(props);

                    // We need a counter because with Fiber cWM may be called multiple times
                    this.count += 1;

                    if (this.count === 1) {
                         styles.use();
                    }
                }

                componentWillUnmount() {
                      styles.unuse();
                }

                render() {
                    return <BaseComponent {...this.props} css={styles} />;
                }
            }

            return componentWithStyles;
        };
    }

@gaearon
Copy link
Collaborator

gaearon commented Apr 1, 2018

Don’t keep a counter like this. That’s not the intended migration strategy. If it’s nowhere in our blog post it’s probably not the recommended way. 🙂

You should move code with side effects (like your example) into componentDidMount. If you have further problems please share a reproducing case.

@bvaughn
Copy link
Contributor

bvaughn commented Apr 1, 2018

What do styles.use() and styles.unuse() do? Can you call styles.use() from componentDidMount / componentDidUpdate instead?

What happens if you call styles.use() more times than styles.unuse()? Because as long as you're doing this from the a lifecycle like the constructor or componentWillMount- this is possible.

@budarin
Copy link
Author

budarin commented Apr 1, 2018

@bvaughn

styles.use() - add styles into head <style> tag, unuse() - remove particular style tag from head

of course I can move code to componentDidMount, but there appears a blinking effect.

I suspect that the problem in an unequal numbers use() and unuse() in case of code in constructor

@bvaughn
Copy link
Contributor

bvaughn commented Apr 1, 2018

of course I can move code to componentDidMount, but there appears a blinking effect.

Hm...this sounds unexpected, if we're talking only about client-side rendered React. Adding the style in componentWillMount should really be no different than adding it in componentDidMount (assuming sync rendering- since both are called in the same tick).

If you're talking about server rendering too, then it would make sense- since componentDidMount isn't invoked on the server. (So I guess it would mean your initially rendered payload wouldn't include styles- they'd only get added after hydration.)

Sorry if I'm overlooking something silly. I don't have much experience with server rendering.

@gaearon
Copy link
Collaborator

gaearon commented Apr 1, 2018

of course I can move code to componentDidMount, but there appears a blinking effect.

Please share a reproducing example.

@budarin
Copy link
Author

budarin commented Apr 1, 2018

@gaearon
I'm sorry - moving code from componentWillMount to componentDidMount does not cause blinking effect - it's my debugging logging code brings delay

the problem remains with the server rendering

@bvaughn
Copy link
Contributor

bvaughn commented Apr 1, 2018

@budarin Can you clarify why using UNSAFE_componentWillMount (or the constructor) for server rendering- as mentioned above- does not work for you?

@budarin
Copy link
Author

budarin commented Apr 1, 2018

@bvaughn
as I understood UNSAFE_componentWillMount is a temporary solution and a dirty hack - it will be removed in the future )

@gaearon
Copy link
Collaborator

gaearon commented Apr 1, 2018

What exactly do you need to do on the server? You can’t attach a stylesheet there because there is no DOM. Can you provide a complete example we can discuss?

@budarin
Copy link
Author

budarin commented Apr 1, 2018

@gaearon
On the server side use() calls dispatch behind the scene and puts styles into the store where we can collect them and form critical css.
Moving the code to a constructor does not cause problems on server side.
The only problem is on client's side - possibly inconsistent count of calls use() and unuse() with moving code to a constructor.

I don't know what I need in this situation - I only pointed you to an existing problem and ask you to help me to resolve it )

@gaearon
Copy link
Collaborator

gaearon commented Apr 1, 2018

So use does different things on client and server side? That helps, thanks.

I think for now the best solution would be to call it in componentDidMount() unconditionally, and call it in the constructor but behind a server-only check.

Longer term we may provide a separate server-only hook that you can replace constructor with: reactjs/rfcs#8

@budarin
Copy link
Author

budarin commented Apr 1, 2018

@gaearon

thanks, that solved my problem.

@budarin
Copy link
Author

budarin commented Apr 1, 2018

@gaearon

May I ask you in this thread to not create another one for a new lifecycle?

Where to move code from componentWillReceiveProps when I need to access to the instance and call a method for computing next state or form state which is based on instance props?

For an example:

    componentWillReceiveProps(nextProps, nextContext) {
        const { computedMatch, location, path, strict, exact, sensitive } = nextProps;

        this.setState(() => ({
            match: Route.computeMatch(
                { computedMatch, location, path, strict, exact, sensitive },
                nextContext,
            ),
        }));
    }

move code to a new method getDerivedStateFromProps is an impossible due to the presence of mandatory of context which is a property of an instance not a declaration.

moving code to componentDidUpdate generates an infinite loop

    componentDidUpdate(prevProps, prevState, snapshot) {
        const { computedMatch, location, path, strict, exact, sensitive } = this.props;

        this.setState(() => ({
            match: Route.computeMatch(
                { computedMatch, location, path, strict, exact, sensitive },
                this.context,
            ),
        }));
    }

@gaearon
Copy link
Collaborator

gaearon commented Apr 1, 2018

You can fix the infinite loop by comparing this.props to prevProps (first argument) and only calling setState when particular props you’re interested in have changed.

Alternatively you can move the context reading code into a component above this one, and pass context as a prop to it. Then you would be able to access it in the static lifecycle.

@budarin
Copy link
Author

budarin commented Apr 1, 2018

@gaearon
Unfortunately, any prop is interested - so this check will not take needed effect.
the only suited way - to move context. Thanks for the idea!

@budarin
Copy link
Author

budarin commented Apr 1, 2018

@gaearon
one more thing with transition to a new lifecycle - moving from componentWillReceiveProps to componentDidUpdate (due to he need to have access to instance fields and methods) will double render cycle - after changing any props and rendering we force component to render again in componentDidUpdate if needed.

It should be mentioned in transition guide.

in our project there are a lot of places where in componentWillReceiveProps we call an instance methods or access an instance fields
So the transition will degrade the performance of app rendering.

@gaearon
Copy link
Collaborator

gaearon commented Apr 2, 2018

Again, it’s hard to discuss without specific examples. The primary transition path when you need to change state in response to new props is to use getDerivedStateFromProps. It won’t cause double rendering. If, for some reason, you need to access instance variables, there is most likely a way to restructure your code so you don’t need to. But I can’t say for sure without seeing it. Finally, side effects should only happen in componentDidUpdate. So sometimes you’d split your existing code in two methods. If you have an example where it seems impossible without making performance slower please share and I’ll take a look.

@budarin
Copy link
Author

budarin commented Apr 2, 2018

Thanks for the explanation

@bvaughn
Copy link
Contributor

bvaughn commented Apr 2, 2018

as I understood UNSAFE_componentWillMount is a temporary solution and a dirty hack - it will be removed in the future )

To clarify, this is incorrect. As the release blog post states explicitly, UNSAFE_componentWillMount will continue to be supported in version 17.

@richard-uk1
Copy link

richard-uk1 commented Jan 29, 2019

Can I ask how the following example would work with functional/hooks.

class MyComponent extends Component {
    constructor() {
        this.field_id = idGenFn();
    }

    render() {
        return <>
            <label for={this.field_id}>Label</label>
            <input id={this.field_id} />
        </>;
    }
}

I need a way to run the function once for each instance of a component, but only before the first render. It's kinda near the boundary between a pure function and a function with side effects. I need the ID before I render, or else I will have to render the component twice.

If it were more of a side effect (e.g. it takes time), it would make sense to render a loading state and then re-render, but since this side-effect is resolved instantly and predictably (the ids should be the same between runs (question: is this true?)), it seems silly to re-render.

EDIT I guess the functional programmer would say "genIdFn isn't pure, use useEffect". I'm gonna do this for now.

@TrySound
Copy link
Contributor

@derekdreery Like this

const useIdGenFn = () => {
  const idRef = React.useRef(null)
  if (idRef.current == null) {
    idRef.current = idGenFn()
  }
  return idRef.current
}

@richard-uk1
Copy link

Aah a custom hook! Thankyou!

@gaearon
Copy link
Collaborator

gaearon commented Jan 29, 2019

https://reactjs.org/docs/hooks-faq.html#is-there-something-like-instance-variables

@sweet-smail
Copy link

I wonder if react-16, when rendering on the server side, will the client also execute the componentDidMount method?
client-Component
image
server-render
@TrySound
image
result
image
That's not right.

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

No branches or pull requests

7 participants