5 ms·

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 us

by dvt 9y ago

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-componentwillmount-vs-componentdidmount/4287/10 https://discuss.reactjs.org/t/constructor-vs-componentwillmo...

AWebOfBrown 9y ago
> 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.
- dvt 9y ago
  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.
  - AWebOfBrown 9y ago
    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.
    - dvt 9y ago
      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-mocking-constructor-using-sinon https://stackoverflow.com/questions/14569499/javascript-mock...
      - davedx 9y ago
        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++
        danabramov 9y ago
        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 https://news.ycombinator.com/item?id=16696836
danabramov 9y ago
>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.
- smashed 9y ago
  > 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.

by dvt 9y ago

AWebOfBrown 9y ago

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

dvt 9y ago
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.
- AWebOfBrown 9y ago
  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.
  - dvt 9y ago
    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-mocking-constructor-using-sinon https://stackoverflow.com/questions/14569499/javascript-mock...
    - davedx 9y ago
      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++
      - danabramov 9y ago
        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 https://news.ycombinator.com/item?id=16696836

danabramov 9y ago

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

smashed 9y ago
> 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.