Comment by dvt | Hacker News Reader

Comment by dvt | Hacker News Reader

Parent Full thread

My apologies if I came off harsh, I have a love-hate relationship with React.

> I totally see what you mean but this particular lifecycle is for a very rare use case.

It's not that rare. Consider a route change where some state needs to be preserved or, like the article mentions, a scroll position change (or reset!).

> You're right—but this solution doesn't solve async data fetching. It only works when the data is synchronously available, and per the post, in that case you can already use the constructor.[3]

I just want to point out how fundamentally flawed this is. The constructor is meant to initialize internal and initial state of objects. Making a data/network request in the constructor is beyond bizarre. In fact, the general consensus was that doing that sort of thing in the constructor is kind of an antipattern[1].

> The mentioned “suspense” API is both much less verbose and actually was designed with server rendering in mind. It would let us fetch data on the server and on the client and suspend rendering until the data is ready (and show a placeholder if it takes too long).

I'll have to check out suspense, but I feel that it's yet another API that reinvents the wheel. We'll see.

[1] https://discuss.reactjs.org/t/constructor-vs-componentwillmo...

> I just want to point out how fundamentally flawed this is. The constructor is meant to initialize internal and initial state of objects. Making a data/network request in the constructor is beyond bizarre. In fact, the general consensus was that doing that sort of thing in the constructor is kind of an antipattern.

Note that he said synchronous data fetching, ie a local cache. If you are querying a local cache, this enables you to grab data before the first render (saving a re-render) which is more obviously beneficial on low-power, slow-to-render devices.

It CAN also be used to initiate a network request, but this is discouraged because it can fire multiple times: meaning you should only do this if the endpoint is idempotent and you can live with excess requests. It may also complicate testing.

Bit OT, but I generally think "that's an anti-pattern" criticism isn't a great contribution unless you're specifying what the problem is.

My criticism was in the context of SSR (in other words, there is no "local cache").

> Bit OT, but I generally think "that's an anti-pattern" criticism isn't a great contribution unless you're specifying what the problem is.

I thought I was pretty clear: putting that kind of thing in the constructor is terrible practice and goes against common sense.

I mean detailing specifically what problems arise. To my mind, calling something a 'terrible practice ... against common sense', particularly when another party (with expertise no less) disagrees, is just a lazy put-down.

I can be clearer if you need me to, but I think this is very common knowledge: the advice of "stick it in the constructor" abuses constructors and makes them do something they weren't meant to do. Incidentally, abusing constructors also sometimes breaks testing. Constructors (in virtually all languages) are notoriously difficult to test as is[1].

[1] https://stackoverflow.com/questions/14569499/javascript-mock...

Yup... don't have side effects in constructors. That's the whole point of other lifecycle methods... this is a practice as old as C++

Please note we are not suggesting side effects in constructors. I’m sorry if my previous comments gave you that impression.

I elaborated here: https://news.ycombinator.com/item?id=16696836

Thanks for the clarification and for pitching in on this thread. FWIW I really appreciate the thought and effort the React team puts into changes.

>My criticism was in the context of SSR (in other words, there is no "local cache")

Why would there be "no local cache" in the context of SSR?

>I thought I was pretty clear: putting that kind of thing in the constructor is terrible practice and goes against common sense.

Those are adjectives, the parent asked for technical arguments.

> Why would there be "no local cache" in the context of SSR?

Because there's no browser?

And who said the application server can't have local cache?

Because that's an insane architectural decision. You already have a database.

>It's not that rare. Consider a route change where some state needs to be preserved or, like the article mentions, a scroll position change (or reset!).

In my anecdotal experience this code is still pretty rare (compared to overall number of components). You don't need this hook to reset scroll position (`componentDidUpdate` alone is sufficient) or maintain it (browser does this automatically if the inserted DOM height is enough to cover it).

The use case for `getSnapshotBeforeUpdate` refers to cases where you have a list that needs to track its current position during offscreen insertions (for example, new items loaded above in the feed) or a list that “catches up” with new content (for example, a message thread that auto-scrolls to the last message unless you’re in the middle of a conversation).

>The constructor is meant to initialize internal and initial state of objects. Making a data/network request in the constructor is beyond bizarre.

I am not suggesting to do network requests in it. Please see the link I pointed to: it shows initializing initial state. Not data fetching.

To be clear: today in React there is no way to asynchronously fetch data on the server. It’s just not supported (and we’re aware it’s an unfortunate limitation). So if the data is already available synchronously (which in your case it seems to be because you said you were able to use `componentWillMount` for this which is also synchronous on the server), then you can synchronously read it in the constructor. If it’s not available synchronously, then there’s nothing `componentWillMount` gives you anyway.

In either case, this limitation is exactly what motivates our work on React Suspense (which will support waiting for data both on the server and on the client). But to support this API, we need to make component compatible with async rendering first. Which is what this blog post is all about.

>I'll have to check out suspense, but I feel that it's yet another API that reinvents the wheel. We'll see.

You’re welcome to start watching the RFC repository and comment when we post more technical details about it. We think it adds a capability that’s always been missing in React (pausing rendering while keeping the screen consistent) and I hope you’ll find it useful.

> To be clear: today in React there is no way to asynchronously fetch data on the server. It’s just not supported

What we've done for the past few years is trigger async fetches in componentWillMount on the server and perform multiple renderToString.

This requires some instrumentation so you can monitor that all the data has loaded and your tree is fully rendered.

Removing componentWillMount will force us to rewrite everything to the suspense API. I hope that transition won't cause too many headaches.

>perform multiple renderToString

The downside of this is that the cost of rendering is multiplied. We know it’s an existing workaround but it’s not a good long term solution. We want to enable something better.

>Removing componentWillMount will force us to rewrite everything to the suspense API.

This is not accurate. The blog post states that `UNSAFE_componentWillMount` is here to stay so you can keep using that until we have a first-class data fetching solution that works both on the client and the server. You can also use the constructor for this (it’s technically equivalent to `componentWillMount`).

While it contradicts what I said earlier (avoid side effects in constructor) you’re already using a hack (rendering twice) and I’d argue this is no worse as a temporary solution. Don’t forget that `componentWillMount` is equivalent to the constructor for all intents and purposes, and only exists because it predates ES6 classes (and the ability to define a constructor). So you were already doing side effects in what is effectively a constructor.

Still, the upcoming Suspense API mentioned in the blog post is the intended long term solution to this. It will work both on the client and the server, and it will allow you to remove the double rendering hack. But to prepare for Suspense, we need to make components compatible with async rendering (which is what the blog post is all about).

We added a section to the blog post which I hope clarified it:

>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.

So you can keep using `UNSAFE_componentWillMount` or constructor for now, and migrate to Suspense when it’s ready (and render your app in a single pass).

Thanks for the detailed explanation. Makes sense and I will keep an eye on the upcoming suspense API.

Reply on news.ycombinator.com