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

RFC: React Hooks #68

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
@sebmarkbage
Collaborator

sebmarkbage commented Oct 25, 2018

In this RFC, we propose introducing Hooks to React. See the RFC and the documentation for more details.

https://reactjs.org/docs/hooks-overview.html

View formatted RFC

(The talk video has now been published too.)


Nov 16 Update by @sophiebits:

See @sebmarkbage’s comment #68 (comment) for a detailed response to the preceding comments that covers various tradeoffs and articulates why we’ve landed on the current design. If you have something to say, please check that it is not answered there already.

RFC: React Hooks
In this RFC, we propose introducing *Hooks* to React. See the RFC and the documentation for more details.

https://reactjs.org/docs/hooks-overview.html
@grabbou

This comment has been minimized.

grabbou commented Oct 25, 2018

Question about persisting values of useState - is the preferred way to create a custom usePersistedState hook that calls into storage mechanism?

I was wondering if there was a way to batch the write operations in environments such as React Native and how did you approach that problem at Facebook during your initial adoption?

@pcmaffey

This comment has been minimized.

pcmaffey commented Oct 25, 2018

Is this the right place for feedback? First of all - amazing work. I love where this is going with composability. My big question is about performance of Hooks.

Are all these function hooks redefined and recalled on every render?

@pheuter

This comment has been minimized.

pheuter commented Oct 25, 2018

Just watched the React Conf keynote, awesome presentation, super excited to give hooks a shot!

Not sure if this is the right place to ask but was wondering how function components that make use of hooks affect server-side rendering. In useEffects() for example, will errors be thrown when accessing browser apis like window during ssr?

@pheuter

This comment has been minimized.

pheuter commented Oct 25, 2018

@pcmaffey Looks like this FAQ entry addresses your performance concern.

@JoshuaKGoldberg

This comment has been minimized.

JoshuaKGoldberg commented Oct 25, 2018

Very exciting, this looks awesome!

Questioning the tuple return type:

const [count, setCount] = useState(0);

Why an array tuple instead of an object containing count and setCount members? Is this for performance (and if so, do we have numbers to show browsers or Node behaving strongly differently)? It feels like leaving the returned object open to adding more fields might make it easier to remain backwards compatible in the future, similar to how APIs often end up taking in single objects as a parameter rather than long lists of parameters.

const { value, set } = useState(0);

Edit: yes, per @jaredLunde's comment, the properties would need constant names.

@jaredLunde

This comment has been minimized.

jaredLunde commented Oct 25, 2018

@JoshuaKGoldberg How would you customize those object properties? Array is the cleanest way it works with custom naming.

@alexeyraspopov

This comment has been minimized.

alexeyraspopov commented Oct 25, 2018

Why an array tuple instead of an object containing count and setCount members?

You can assign names you need. Object destructuring requires you to put specific keys. You may end up having something like

let { state: name, setState: setName } = useState('Name');
let { state: surname, setState: setSurname } = useState('Surname');

Which is not the case with tuples.

@jamesplease

This comment has been minimized.

jamesplease commented Oct 25, 2018

I'm still familiarizing myself with this API, but I'm optimistic about these changes. Great work, React team!

My main concern with hooks is related to the learning curve. There are some APIs introduced that new (and seasoned) developers may not immediately pick up on.

One in particular that stands out the most to me is useEffect(myEffect, []). It's not very expressive. For those who haven't read the docs, what do you think this does? I'd be surprised if someone could guess.

For those who are still familiarizing themselves with the API, it calls the effect only on mounting and unmounting, and not on updates (docs here).

I know that this proposal introduces a lot of new functions, but it might be worthwhile adding another one that functions the same as useEffect(myEffect, []). As for the name? I'm not sure. Perhaps something like useMountEffect()?

@eps1lon

This comment has been minimized.

eps1lon commented Oct 25, 2018

It is a very early time for Hooks, so some integrations like DevTools support or Flow/TypeScript typings may not be ready yet.

Will I be able to inspect what hooks a component is using?

Having an object with named properties as state made it easy to see what's what. I guess this will be hard with the new useState?

@benjamn

This comment has been minimized.

benjamn commented Oct 25, 2018

@eps1lon According to the FAQ, useState is backed by a list of anonymous memory cells:

There is an internal list of “memory cells” associated with each component. They’re just JavaScript objects where we can put some data. When you call a Hook like useState(), it reads the current cell (or initializes it during the first render), and then moves the pointer to the next one. This is how multiple useState() calls each get independent local state.

So if you ever find yourself trying to debug the underlying state, you're probably going to be working with a linked list of unnamed values. To figure out which memory cell is which, you'll have to refer back to the order of your useState calls.

Full disclosure: I'm planning to open an issue today to discuss a system for providing meaningful names for your useState calls, which should significantly relax the current usage restrictions, and eliminate the need for a linter to enforce those rules.

@spicydonuts

This comment has been minimized.

spicydonuts commented Oct 25, 2018

Positional effects (using any of these use* functions is an effect, even just to read state) worry me. It seems easy to get wrong, and doing so might not present until production. For the same reason it also seems like it'll be extremely difficult to introduce to beginners -- "the magic function works here but not there".

Using Symbols instead of positional keys would make it easier to understand and harder to get wrong:

const $count = Symbol("count");

function MyComponent() {
  const [count, setCount] = useState($count, 0);
  ...
}

Edit for clarification: I don't mean to say the Symbol would be a global key -- it would still use the current context (the Fiber, I assume) as the primary key and the Symbol as the secondary key (where the effectful call order is currently being used).

Another edit for followup: After playing with hooks a bit more I don't feel as strongly about this as I thought I would. It's not perfect, but you could also pass the wrong Symbol in this alternative as well. Also seeing the eslint plugin catching on relieves the worry I had a bit too.

@adamhaile

This comment has been minimized.

adamhaile commented Oct 25, 2018

Just curious, are React hooks at all inspired by S and Surplus? There are similarities down to even the names and terminology used. S has S.effect() and S.cleanup(), and useState() is analogous to S's S.data().

For instance, here's the hooks example in Surplus:

function Example() {
  const count = S.data(0); // S.data instead of React.useState

  S.effect(() => { // S.effect instead of React.useEffect
    document.title = `You clicked ${count()} times`;
  });

  return (
    <div>
      <p>You clicked {count()} times</p>
      <button onClick={() => count(count() + 1)}>
        Click me
      </button>
    </div>
  );
}

In fact, I used almost this exact same example in the Surplus FAQ to explain how functional components could have state -- the very same issue hooks are solving for React.

I'm the author of S and Surplus, and reading the hook docs gave me a strong case of déjà vu! The similarities may be coincidental -- we're probably reading the same upstream sources. But if there was any influence, I thought that would be cool :).

@alexeyraspopov

This comment has been minimized.

alexeyraspopov commented Oct 25, 2018

@alqamabinsadiq, sorry if my comment was somehow confusing. This is not how the hook intended to be used, I was making an example of additional effort required for using object destructuring. This is how the correct code looks like:

let [name, setName] = useState('Name');
let [surname, setSurname] = useState('Surname');

Less effort to write, less effort to read.

@sophiebits

This comment has been minimized.

Member

sophiebits commented Oct 25, 2018

@adamhaile To my knowledge we weren’t influenced at all by Surplus (this is my first time seeing it). It’s true the “effect” name is similar, but both names are descriptive, so I guess it shouldn’t be too surprising that the names overlap.

@sebmarkbage

This comment has been minimized.

Collaborator

sebmarkbage commented Oct 25, 2018

@benjamn @spicydonuts If you decide to publish a follow up RFC with named hooks which doesn’t have the conditionality constraints, then be sure to cover other pitfalls that you may also get. In the current proposal there is clearly one thing that will unexpectedly break when you don’t have names. However there are other things that might become confusing too. Such as that an effect doesn’t cleanup and refire when it’s in a conditional. So even with names you might find it best practice to avoid conditionals.

@spicydonuts

This comment has been minimized.

spicydonuts commented Oct 25, 2018

I see what you mean. Hmm.. that's unfortunate 🤔

@rudolfolah

This comment has been minimized.

rudolfolah commented Oct 25, 2018

I like the idea, but I think the naming doesn't really reflect what's going on here. The term hook in other contexts, such as web hooks or Emacs, implies that something happens at a particular point in time and calls out to some other functions/methods to make it happen.

In the words of the Github docs for Web hooks:

Webhooks allow you to build or set up GitHub Apps which subscribe to certain events on GitHub.com. When one of those events is triggered, we'll send a HTTP POST payload to the webhook's configured URL.

For example web hooks are called whenever Github detects a branch has been created. They're called in Emacs whenever a file is loaded or a particular mode (for syntax highlighting + shortcuts) is activated.

In React, the only hooks I can think of at the moment are the componentDidMount and componentWillUnmount functions, since they're part of a lifecycle and call out to some other function (as part of being overridden in a Component class)

From the React Hooks documentation it looks like these are managers:

  • useState manages and coordinates state
  • useEffect manages and supervises side-effects
  • useContext manages context

Other options for naming:

  • manager
  • supervisor
  • coordinator
  • handler

I'm sure there are other, better names other than "hook" which can be used.

Also, the naming of the methods don't need to change since none of them seem to have the word "hook" in them, the naming change would just affect the rest of the documentation/examples.

@leebyron

This comment has been minimized.

leebyron commented Oct 25, 2018

Loving this new API, but I have one primary concern - where the hooks come from.

I'm concerned that these hook functions are provided by the top level react module and imply (or require) global state to work. Ideally, hooks and effects functions could be provided to the functional component when it's executed.

So while this example as illustrated by Dan requires from the module:

const { useContext } = require('react')
const SomeContext = require('./SomeContext)

function Example({ someProp }) {
  const contextValue = useContext(SomeContext)
  return <div>{someProp}{contextValue}</div>
}

Was it considered to pass the hooks into the functional component?

Perhaps looking something like:

const SomeContext = require('./SomeContext)

function Example({ someProp }, hooks) {
  const contextValue = hooks.useContext(SomeContext)
  return <div>{someProp}{contextValue}</div>
}

This means functional components which use hooks are (Props, Hooks) => ReactNode. While this is still not "pure" because hooks have effects and are not pure, at least the renderer could provide the appropriate hooks per call-site instead of relying on global state. I could imagine that the Fiber renderer might have different implementation of hooks than some potential future renderer, a server-side renderer, a test renderer with mocked effects, etc.

Thoughts?

@KARTIK01

This comment has been minimized.

KARTIK01 commented Oct 25, 2018

for new react developers, useState hooks will be a magic.

@ntucker

This comment has been minimized.

ntucker commented Oct 25, 2018

How does this interact with React.memo()? Does it just shallow check props and ignore state and context. If not, how does it know what state and context to check?

@alexeyraspopov

This comment has been minimized.

alexeyraspopov commented Oct 25, 2018

I'm concerned that these hook functions are provided by the top level react module and imply (or require) global state to work. Ideally, hooks and effects functions could be provided to the functional component when it's executed.

@leebyron, I think, the way how it works, is by using an incapsulated access to the current rendering fiber that holds the state and effects. The API may look like something "global" happens under the hood, but it is still scoped to the specific fiber that reflects the component that is rendering at the moment.

@benjamn

This comment has been minimized.

benjamn commented Oct 25, 2018

@leebyron Alternatively, functional components could be invoked by the React internals with a meaningful this object, containing any number of useful methods.

@david

This comment has been minimized.

david commented Oct 25, 2018

What's the advantage of this approach over a HOC-based one such as the one used, for example, in recompose?

@sebmarkbage

This comment has been minimized.

Collaborator

sebmarkbage commented Oct 25, 2018

@leebyron Conceptually they’re algebraic effects. It just happens that the engine (a pointer to the current “handler”) lives in the react package. Note that the actual implementation doesn’t live in the react-package. So anyone can implement their own dispatcher. Even inside a react render. If this is a popular pattern you could just have a general purpose pattern for this like “dispatcher” package or something.

Just like context there doesn’t have to be one. In multi-threaded environments there can be several.

We considered passing it but then you would have to pass it through multiple layers of indirection. We already know this to be a pain (which is why we have context to begin with) and for something as light weight as custom hooks it is even more so.

@jaredLunde

This comment has been minimized.

jaredLunde commented Oct 25, 2018

@david acdlite/recompose@7867de6

@sebmarkbage

This comment has been minimized.

Collaborator

sebmarkbage commented Oct 25, 2018

@benjamn The this was actually in the first idea and later dropped because it added weirdness and got even weirder when composed with custom hooks. Since they would need to change to a different variable or use .call.

@probablyup

This comment has been minimized.

probablyup commented Oct 25, 2018

Are there any concerns about the interaction of transpilers with state hooks? The order of function calls could conceivably be rearranged by a transpiler or minifier in a way that differs between an SSR and client-side bundle.

@brennancheung

This comment has been minimized.

brennancheung commented Nov 16, 2018

This new API looks very pragmatic for the simple use cases that are presented but I'm getting some red flags because this breaks many functional programming (FP) and even general programming paradigms.

Adherence to FP paradigms tends to facilitate composition. Hooks seem to deviate from it so my fear is that certain types of composition and higher order functions (HOF) will not be possible. Additionally, the novelty of this approach increases the chances of uncertainty and conflict down the road.

Generally speaking, these are my concerns:

  1. This blurs the line between stateless functions and stateful "classes".
  2. This breaks idempotency—calling the same function multiple times can yield different results.
  3. The positional requirement when using useState seems unnecessary if we can just pass in a key. const [name, setName] = useState('name', 'defaultValue')
  4. Introducing side-effects into functions makes testing harder.
  5. This is abusing the JS language and creates an additional learning curve compared to native JS paradigms. Is it possible to use something that takes advantage of native language constructs?
  6. It's a React only convention that prevents code interoperability with non-React libraries. A pure JS solution would be greater for the community at large if non-React libraries could be used within React applications—and vice versa.

Pushing side-effects into small sections of code allow a greater percentage of the codebase to remain pure. This helps with understanding, composition, and testing. This proposal seems to be moving in the opposite direction.

The 2 primary mechanisms to limit side-effects, and the consequent problems, are typically:

  1. Passing the variable part in as a function parameter (injection). e.g., (const Component = (props, context, state) => { ... }).
  2. Creating a closure that contains the variable part. e.g., (const Component = state => props => { ... })

I'm wondering if we can have the useState semantics with either/both of these techniques.

How would testing work with useState? How would we get the component into a certain state to verify its behavior? If it was passed in, testing would be easy. But when the state is encapsulated within the function this makes injection and mocking more difficult.

@dantman

This comment has been minimized.

dantman commented Nov 16, 2018

This discussion feels like it's going in circles.

@transitive-bullshit

This comment has been minimized.

transitive-bullshit commented Nov 17, 2018

(previous FB dev if that means anything these days 😄)

I can't help but think that React hooks wouldn't be a thing if the React team had invested heavier into decorators, which afaik gives you the same composable functionality and ability to have custom reusable hooks using the same APIs as built-in hooks. (note: I am not referring to legacy decorators).

The main advantage of using decorators over hooks would be that they are a bit more natural and built into the ES standards, as well as closer parity with how other programming languages solve for this type of functionality.

My main concern with hooks is not the reliance on implicit ordering within render functions (though that is awkward), but rather that they introduce & encourage two ways of achieving the same thing with React's public APIs. This will lead to confusion and inconsistency in tutorials for beginners over how to achieve something because there will always be two functionally equivalent answers. This is analogous to the early days of React and React Native where all the official examples as well as the litany of online articles & tutorials had both ES5 createElement and ES6 class versions that greatly hindered one's ability to learn the core primitives that React was exposing. This has since been mitigated with CRA and 99% of React apps using ES6 and a bundler, but I fear that the React team hasn't learned from this previous lesson.

With all that being said, I'd like to applaud the React team for having the courage to try and innovate with such a large change. I'm just worried over the long-term effects that hooks will have over React's increasing API surface and duplicate ways of achieving the same thing, which in any API design regardless of level of abstraction is a red flag. Given the React community's constant need to "jump on the bandwagon", I'm afraid that any potentially damaging ramifications of this split have already been introduced :sigh: and that the majority of the React community has reacted more with confirmation bias of jumping onto anything the React team introduces than what the React team actually wanted, which was an unbiased view of an experimental proposal.

One concrete question I have is: are there any functional differences in what hooks allow in their current incarnation than if they had been implemented via ES decorators?

@FredyC

This comment has been minimized.

FredyC commented Nov 17, 2018

@transitive-bullshit I've read the first paragraph and at the moment I saw decorators I've stopped reading. Sorry man, but would you really build a major functionality on top of experimental decorators? Who knows if it will ever be finished. What would happen if it doesn't? This is no Angular, we doing serious business here 😆

@brennancheung Check the first comment, you will find there a link to comment that explains most of your concerns.

@Kovensky

This comment has been minimized.

Kovensky commented Nov 17, 2018

@interphx

This comment has been minimized.

interphx commented Nov 17, 2018

@transitive-bullshit
As far as I understand, with decorators we wouldn't be able to easily pass return values of one hook into another. Another issue is typing: there is currently no mechanism in e.g. TypeScript that would allow us to gather the types of injected values from decorators and pass them to the function (assuming we use @decorator syntax). In general, I think a decorator-based solution is possible, but would eventually introduce more syntax noise.

@transitive-bullshit

This comment has been minimized.

transitive-bullshit commented Nov 17, 2018

@FredyC @PinkaminaDianePie this is a misconception that is unfortunate due to the TC39 process that have left many devs with knee jerk reactions to decorators. Decorators are now at stage 2, whereas you're referring to legacy decorator support.

@Kovensky these minification concerns are certainly valid, but they shouldn't have much pull over high-level framework design decisions since they can and should be solved lower in the abstraction hierarchy (e.g., during bundling / transpilation).

@interphx these are very reasonable & valid concerns. Any solution is certainly going to have tradeoffs; I'm just curious what these concrete tradeoffs look like with hooks implemented via the current implicit ordering method versus the explicit ordering that decorators mandate.

@sophiebits

This comment has been minimized.

Member

sophiebits commented Nov 17, 2018

@FredyC Please don’t put down other people or solutions by saying things like “we doing serious business here 😆”.

@transitive-bullshit Decorators is certainly one idea that we thought of. The critical difference is it doesn’t allow you to take the return value of one hook and pass it as an argument to another. This is one part of the design that we think is very important, especially as you get down to building more complex hooks, such as on top of useContext and useMemo. Even accessing state in an event handler would likely be nontrivial. Sure – perhaps other compromises in the API could be made to allow that in a limited way, but we haven’t seen a version of that that is as straightforward and powerful; if there is a specific API proposal you have in mind that doesn’t have these downsides please feel free to post it. I’ll also mention that if the hook results were passed as arguments to the function (the most natural result of using decorators) then long lists of hooks can get unmanageable quick since you need to match up the order of decorators with arguments precisely. I’d say this is a secondary concern (the one I mentioned above is more fundamental) but a real one nonetheless.

@PinkaminaDianePie

This comment has been minimized.

PinkaminaDianePie commented Nov 17, 2018

@transitive-bullshit
Sorry bro, but personally for me decorators looks like worst proposed solution in this thread.

  1. They are much more complex than normal functions.
  2. They require usage of classes, while long term direction is moving to functions. (ReasonML is "ES2030" for Facebook, react/redux was heavily FP-influenced, flow have a very weak support of classes compared to FP etc), so it's kinda strange to propose OOP based solution. If you really worked in FB you should know it much better than me, so it's even strange to hear such proposals.
  3. Current proposal fit FB trends - more and more FP stuff, if it will work they could work on it further, like introducing first-class effects, to have a clear distinction between pure and unpure code.
  4. Current proposal use as least ES features as possible, and it was big advantage of react stack for long time. You need to know only functions, destructuring, jsx and a bit of other stuff, and its enough to create complex and scalable app, which will be easier to support and reason about. Mix of different patterns and ES features made some other frameworks hard to learn and maintain, especially for newbies. React is extremely easy to learn, and small pool of used ES features is one of the reasons.
  5. Small amount of used patterns and ES features made it easier to optimize the code, both for devs, react team, and browser engine.
@transitive-bullshit

This comment has been minimized.

transitive-bullshit commented Nov 17, 2018

Thanks for the excellent breakdown and for taking the time to answer my question @sophiebits ❤️

@AlexGalays

This comment has been minimized.

AlexGalays commented Nov 17, 2018

I'm really glad React doesn't even begin to touch decorators. They're bad for typesafety, composition, debugging, etc.

@timkendrick

This comment has been minimized.

timkendrick commented Nov 17, 2018

@sophiebits

we haven’t seen a version of that that is as straightforward and powerful; if there is a specific API proposal you have in mind that doesn’t have these downsides please feel free to post it

I feel that I suggested a straightforward and powerful API proposal that gels well with the rest of the React API – i.e. React.withHook(hook, component) – in this comment (elaborated in this response).

While it's very slightly more verbose than the current proposal, it's far less likely to provoke any of the strong negative responses from FP purists that the current form has received, and is purely additive (i.e. could be implemented without any changes to the existing core). I'd be very eager to hear the team's views – I know how busy you guys must be so I don't expect a detailed appraisal; even a simple yes/no/maybe would be fine.

If it's of interest then I can gladly code it up for people to play around / experiment with? Let me know if you think it's an avenue worth pursuing or not.

(Thanks for all the work on this BTW, I'm confident that hooks will be a game-changing addition to the library!)

@FredyC

This comment has been minimized.

FredyC commented Nov 17, 2018

@timkendrick Sorry man, but I don't think it's funny anymore :) I understand you are trying to find some better way, but reminding all over again the same thing is getting kinda old. Please, if you truly believe it your approach, make a proper separate RFC issue where the discussion can be more focused. Thanks :)

@RichieAHB

This comment has been minimized.

RichieAHB commented Nov 17, 2018

@timkendrick your idea seems similar to my initial thoughts on this (in fact I posted something similar to yours above, using plain higher order functions as opposed to decorators). The problem I later realised - as @sophiebits mentions - is to build hooks that compose other hooks, I.e. custom hooks. At this point, threading values between hooks can become pretty cumbersome with an API like this.

I’d encourage you to play around and see whether your idea becomes unwieldy at this point (as mine did). If it doesn’t then - again, as mentioned above - I for one would love to see it!

Personally, I still have some qualms around hooks regarding testing, no early returns and some of the things already mentioned above (not least some “dogmatic” concerns, I’ll be honest). However, when playing around with non-trivial examples, specifically custom hooks, I began to see why they’d chosen this approach and I’ve started to come around!

@davidnagli

This comment has been minimized.

davidnagli commented Nov 17, 2018

Hi guys! I have something that I want to add to hooks. I'm not sure if it's already been discussed (it's borderline impossible to read all 1,087 comments at this point), so I'm really sorry if this has already been said.

Anyway, my idea was to add an additional parameter to all built in hooks that are affected by order-of execution that specifies a unique id for each hook call. Instead of only relying on order-of-execution hooks could then rely on this unique id to determine which hook is which.

This parameter would be optional and would probably only be set by tooling such as a babel plugin etc which could automatically add a unique id to each call site. The implementation for how to generate these unique ids can be left up to the tooling (hashes, incrementing counter, etc). The only API requirement would be to make this id a unique value.

As far as I can tell, this would solve all this weird stuff with not being able to call hooks from loops and conditions when people use tooling that supports this.

Thoughts?

tldr; Add an additional parameter to all built-in hooks get rid of the call-order requirement by adding a unique id for each hook call injected via automatic tooling

CC @sebmarkbage

@sophiebits

This comment has been minimized.

Member

sophiebits commented Nov 17, 2018

@timkendrick Thanks for posting a concrete idea. It’s much more helpful than talking about ideas in the abstract. A few thoughts on yours:

  • Syntax is considerably more cumbersome and verbose. I think yours looks mostly manageable, but all other things equal we would prefer the one with less typing because React devs will be writing hook calls many, many times per day.
  • Needing a useProps hook instead of being able to rely on arguments is a little unfortunate IMO. Not only does it feel more complicated (to me, at least), but it has two other quirks: first, it breaks encapsulation for custom hooks (today, we don’t have useProps in part because it’s nice that a custom hook can only access parameters passed to it; it can’t observe or modify the component calling it); second, it means that defaultProps can’t be replaced with JS default arguments as Sebastian mentioned above since these no longer share a scope.
  • I’d argue the transition from a single hook to useHooks to useRelated is not intuitive, especially when you compare to the consistency we have today where (you have to follow the Rules, but) you can easily go from 0 to 1 to 2 hooks, and from there to a custom hook.
  • In useRelated, you still need authors to follow the nonconditionality rules. Your proposal eliminates the need to know these partway but not for all cases.
  • Custom hooks are possible (thank you) but not as easy. In particular, you can’t just copy-paste component code and have it work like you can for extracting a function normally; you need to add the run() falls which is a bit annoying and makes it harder to see the structure.
  • Like decorators, needing to match up the argument order with the hook order can be a bit annoying with long lists.
  • If I’m not mistaken, this is actually less optimizable at runtime and would likely run slower than our proposal because of the extra indirection and allocations.
  • All in all, I don’t believe this is actually easier to learn. I may be wrong but it seems you now need to teach the concept of the hook “thunk” return value as different from the conceptual one you get in the component itself, you need to teach multiple additional hooks, and you still have the rules which are, at a first glance, usually the quirkiest part of hooks today.

Given these downsides I don’t expect that your idea will make it. However I appreciate that you took the time to think it out. Finally, I’ll add that although Hooks “looks impure” today, we don’t anticipate it inhibiting us from really doing anything we want to do with them. Because the Hooks dispatcher can be swapped at runtime we can customize the behavior (ex: for devtools) even though it looks statically bound. In that sense they are still pure – no global side effects other than calling the hook functions on the dispatcher, which if desired can be set up to capture all the calls and return them in a non-side-effectful way.

@timkendrick

This comment has been minimized.

timkendrick commented Nov 17, 2018

Thanks for posting such a thorough response @sophiebits – really appreciate the direct (and prompt!) feedback from a member of the core team.

Looking forward to the arrival of hooks, whatever form that takes!

@jay51

This comment has been minimized.

jay51 commented Nov 18, 2018

I haven't tried react hooks yet but I don't think it's a good idea. simplicity is better than complexity. Introducing a second way of doing something only makes it harder to decide which way to go. I think having one standard way of doing something is better than 2, let alone the increase in the library size. I never had any issues with the class components, in fact, it's much easier to pick up. These frameworks never standardize they keep on changing, for now, good reason. I think I'm going to either switch to Polymer because it's very small and simple or leave web dev completely.

@PutziSan

This comment has been minimized.

PutziSan commented Nov 18, 2018

Thank you very much for the detailed answers from the react team on many points.

I had made a few suggestions in the comments about how to do it with a wrapping function (the first time 20 days ago). I also presented the proposal in more detail in a separate project (together with a second proposal and a working demo). Even after the very good answers so far, I don't notice anything where the current design offers an advantage over the solution with a wrapping function.

If someone were to go into this again, it would be really helpful to understand why it was decided against such an approach.

@FredyC

This comment has been minimized.

FredyC commented Nov 18, 2018

@PutziSan I believe that this comment from @sophiebits pretty much covers most of your proposal. Having access to props in a custom hooks feels awkward to me as suddenly you are tightly coupled to a specific shape of the object.

  • Needing a useProps hook instead of being able to rely on arguments is a little unfortunate IMO. Not only does it feel more complicated (to me, at least), but it has two other quirks: first, it breaks encapsulation for custom hooks (today, we don’t have useProps in part because it’s nice that a custom hook can only access parameters passed to it; it can’t observe or modify the component calling it); second, it means that defaultProps can’t be replaced with JS default arguments as Sebastian mentioned above since these no longer share a scope.
@btraljic

This comment has been minimized.

btraljic commented Nov 18, 2018

Unexpected(?) behavior of reusable hook functions
or
what am I doing wrong?

I am trying to write my own reusable useEffect hook function useEffectOnce() to run only once (on mount and unmount), but it seems impossible. My function is called after every re-render: https://codesandbox.io/s/mjx452lvox

function useEffectOnce() {
  console.log("I am in useEffectOnce");

  useEffect(() => {
    console.log("I am in useEffectOnce's useEffect");
    return () => {
      console.log("i am leaving useEffectOnce's useEffect");
    };
  }, []);
}

function App() {
  const [count, setCount] = useState(0);

  useEffectOnce(); // once!?

  return (
    <div className="app">
      <h3>useEffectOnce (my reusable function)... Wait, once!?</h3>
      <button onClick={() => setCount(count + 1)}>
        You clicked me {count} times
      </button>
    </div>
  );
}

Notes:

  • useEffect() hook inside useEffectOnce() works as expected
  • useEffectOnce(() => {}, []); changes nothing

... and then big surprise, same behavior with reusable useState hook function(!?): https://codesandbox.io/s/2145m37xnr

My function useButton is called after every re-render, and when is first, independent, button clicked.

function useButton(initialCounterValue) {
  const [usebuttoncount, setUseButtonCount] = useState(initialCounterValue);

  console.log("I am in useButton");

  const handleuseButtonClick = () => {
    setUseButtonCount(usebuttoncount + 1);
  };

  return {
    usebuttoncount,
    onClick: handleuseButtonClick
  };
}

function App() {
  const [count, setCount] = useState(0);
  const useButtonCounter = useButton(0);

  return (
    <div className="app">
      <h3>
        useButton (my reusable useState function) is called only when... OMG!?
      </h3>
      <button onClick={() => setCount(count + 1)}>
        You clicked me {count} times
      </button>
      <br />
      <button {...useButtonCounter}>
        (useButton hook) You clicked me {useButtonCounter.usebuttoncount} times
      </button>
    </div>
  );
}
@j-f1

This comment has been minimized.

j-f1 commented Nov 18, 2018

Hooks are run on every render @btraljic. Imagine copy-pasting the hook’s code into your component directly — the console.log would be called each render. Custom hooks behave in exactly the same way.

@FredyC

This comment has been minimized.

FredyC commented Nov 18, 2018

Hooks are run on every render @btraljic. Imagine copy-pasting the hook’s code into your component directly — the console.log would be called each render. Custom hooks behave in exactly the same way.

I think he is concerned that even though he is passing [] to useEffect, it is executed on every render. In the codesandbox there is a clear repro and I don't really see there would be anything wrong with the code.

Nevermind, @edgesoft is right 👇

@edgesoft

This comment has been minimized.

edgesoft commented Nov 18, 2018

@btraljic

function useEffectOnce() {
  console.log("I am in useEffectOnce");

  useEffect(() => {
    console.log("I am in useEffectOnce's useEffect");
    return () => {
      console.log("i am leaving useEffectOnce's useEffect");
    };
  }, []);
}

The useEffectOnce runs on every render as @j-f1 points out but the useEffect inside the useEffectOnce only runs on mount

@edgesoft

This comment has been minimized.

edgesoft commented Nov 18, 2018

@btraljic So if you want to make a reusable useEffect you should simple put the logic in the useEffect and React will take care of it.

useEffect(() => {
   // put all the code that I want to run only ones.
    console.log("I am in useEffect");
    return () => {
      console.log("i am leaving useEffect");
    };
  }, []);

Edit: Your first useEffectOnce is fine. As long as you make all code that should only run on mount in useEffect

@JekaMedvedev

This comment has been minimized.

JekaMedvedev commented Nov 18, 2018

I have a question, when I pass a function for changing the state to a child component, when I call this function from the child I get undefined for isOpenArticles. This is normal? with class component i dont have that problem!

function Articles(props) {
  const [isOpenArticles, changeVisible] = useState(false);
  console.log(isOpenArticles)
  
  const artticlesForRender = isOpenArticles
    ? [{id:123, text: ""}].map(article => {
        return(
          <li key={article.id}>
              {article.text}
             <Childer handle={changeVisible} />
          </li>
        );
      })
    : null;

  return (
    <div>
      <button onClick={() => changeVisible(!isOpenArticles)}>
        {isOpenArticles ? 'Hide articles' : 'Open articles'}
      </button>
      <ul>{artticlesForRender}</ul>
    </div>
  );
}


function Childer ({ handle, handleArticle }) => {
  return <button onClick= {() => handle()}>Hide Me</button>;
};
@Janpot

This comment has been minimized.

Janpot commented Nov 18, 2018

@JekaMedvedev Did you mean to do {() => handle(true)} instead of {() => handle()} or something?

Edit: Reading it again, seems like you wanted to do

<Childer handle={() => changeVisible(false)} />
@JekaMedvedev

This comment has been minimized.

JekaMedvedev commented Nov 18, 2018

@Janpot
omg i am very inconsiderate, thanks!
Just along with the function, it is also necessary to transfer the current state, I forgot about it) It works

<Childer handle={changeVisible} han={isOpenArticles}/>
function Childer({ handle, han }) => {
  return <button onClick={() => handle(!han)}>Hide Me</button>;
};
@FredyC

This comment has been minimized.

FredyC commented Nov 18, 2018

Is total encapsulation of custom hooks a really good thing? It feels great on paper for sure, but sometimes it might be useful to know something about containing component, especially in DEV environment to provide some useful warnings and stuff.

We are trying to improve MobX experience. Currently, there is a high order component necessary to rerender component when any mentioned observables change. However, we also provide means of creating an observable within the component using the hook.

import { Observer } from "mobx-react-lite"

const ObservePerson = observer(() => {
    const person = useObservable({ name: "John" })
    return (
        <div>
            {person.name}
            <button onClick={() => (person.name = "Mike")}>No! I am Mike</button>
        </div>
    )
})

We could make useObservable to track & rerender component based on an observable change, that's not a problem. However, with that being an implicit behavior it would mean that observer attached to such a component would cause another rerender.

If we would be able to somehow share some information (not the actual state) between distinguished hooks, we would be able to provide a meaningful warning so a user is not surprised by a hard to spot performance hit. I am not entirely sure how it would work.

Some issues for a reference if you want more information

@Kovensky

This comment has been minimized.

Kovensky commented Nov 19, 2018

Another concrete example of a benefit of hooks encouraging reusable / recomposable logic. This approach fixed a bug that I had previously worked around using a Context pair to pass around a onResize callback.

interface ElementSize {
  width: number
  height: number
}

function measure(ref: Element | null): ElementSize | undefined {
  return ref !== null ? ref.getBoundingClientRect() : undefined
}

function useElementSize(ref: React.RefObject<Element>) {
  // _don't_ call measure synchronously here even if you somehow can
  // measurement has to be done outside the render phase, either
  // as the resize observer callback or as a layout effect

  const [size, setSize] = useState<ElementSize | undefined>(undefined)
  const handleResize = useCallback<ResizeObserverCallback>(
    () => {
      // NOTE: the ResizeObserverCallback receives a rect[],
      // but it's not measured in the same way as getBoundingClientRect,
      // which would cause unstable layout
      setSize(measure(ref.current))
    },
    [ref]
  )
  const observer = useMemo(() => new ResizeObserver(handleResize), [handleResize])

  useLayoutEffect(
    () => {
      // ensure "current" in the unobserve refers to the same element as observe
      const { current } = ref
      if (current !== null) {
        // The ResizeObserver is supposed to call handleResize on observe
        observer.observe(current)
        return () => {
          // this will correctly unobserve if either the observer
          // or the current changes, and even on unmount
          // no need for an observer.disconnect() 🎉
          observer.unobserve(current)
        }
      }
    },
    [observer, ref.current]
  )

  return size
}

Even if I had used a better HOC approach than that silly context one, I would have needed a <DetectResize> HOC and I would have had to provide my own onResize handler and would have to save the resize result in state.

class UsesDetectResize {
  state = { size: undefined }

  render() {
    return (
      <DetectResize onResize={this.handleResize}>{ref =>
        <div ref={ref}>{/* contents */}</div>
      }</DetectResize>
    )
  }

  handleResize = size => {
    this.setState({ size })
  }
}

Compare with:

function UsesElementSize (props) {
  const ref = React.useRef(null)
  const size = useElementSize(ref)

  return (
    <div ref={ref}>{/* contents */}</div>
  )
}

No need to set up my own event handler or state object, and size will always be current as of the previous update (which is the only way to be aware of any DOM measurement in React anyway).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment