Skip to content
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

useState hook #1453

Open
simonjoom opened this issue Feb 27, 2019 · 25 comments
Open

useState hook #1453

simonjoom opened this issue Feb 27, 2019 · 25 comments

Comments

@simonjoom
Copy link

@simonjoom simonjoom commented Feb 27, 2019

Does inferno compatible with the new react hook useState?

@Havunen
Copy link
Member

@Havunen Havunen commented Feb 28, 2019

No, not yet at least

@hronro
Copy link

@hronro hronro commented Mar 1, 2019

@Havunen
Is there any plans to implement hooks in the future?

@simonjoom
Copy link
Author

@simonjoom simonjoom commented Mar 27, 2019

preact just implemented a hook compat version

https://github.com/developit/preact/tree/master/hooks

i looked on code, maybe can somebody inspired to add this support to inferno?

I would like to use amazing react-spring who use hook system =)

@stylemistake
Copy link
Contributor

@stylemistake stylemistake commented Oct 2, 2019

It seems that hooks aren't that hard to implement. All Inferno hook points are clearly defined, we just need to expose them on an options object, and copy paste, wire up the Preact's file.

@BaibhaVatsa
Copy link

@BaibhaVatsa BaibhaVatsa commented Feb 29, 2020

Hi! I was interested in working on this issue if this is still open and good to go for

@stylemistake
Copy link
Contributor

@stylemistake stylemistake commented Feb 29, 2020

@BaibhaVatsa Yes, I'd really like to see this implemented.

@BaibhaVatsa
Copy link

@BaibhaVatsa BaibhaVatsa commented Feb 29, 2020

Since I am new to this project, would you like to give me any pointers regarding the implementation apart from the link to preact implementation?

@stylemistake
Copy link
Contributor

@stylemistake stylemistake commented Feb 29, 2020

This is a very basic, bare bones setup for hooks (i was coding it inside tgstation repo, which is less than ideal, but this diff will give you the basic idea) :
stylemistake/tgstation@d1b1385

I think these are all the hook points necessary to implement the top level React Hooks API in Inferno. You can use the options object to expose various inferno internals.

Then we have this (preact hooks), what i think is the cleanest implementation of react hooks out there:
https://github.com/preactjs/preact/blob/master/hooks/src/index.js

The main stopper is that you can't use these hooks as is, because:

  • options object is different in preact, they expose their internals differently, and renderer as well works a bit differently;
  • their virtual dom objects are also different, and they put hook metadata into them (__hooks property), to keep track of the lifecycle,

They also have this stinky bit, this is where I personally got stuck:
https://github.com/preactjs/preact/blob/master/hooks/src/index.js#L47-L61

They have a commit queue and a _renderCallbacks on components, and I could not figure out how to reimplement this cleanly using just those hooks exposed via the options object and __hooks prop.

I'm not sure if i have give you more hints on how to approach this issue, I have only scratched the surface myself. If you have questions, contact me in inferno's Slack, I'll be glad to guide you and help where I can.

@marvinhagemeister
Copy link
Collaborator

@marvinhagemeister marvinhagemeister commented Feb 29, 2020

Disclaimer: I'm one of the maintainers of Preact.

They also have this stinky bit, this is where I personally got stuck:
https://github.com/preactjs/preact/blob/master/hooks/src/index.js#L47-L61

The "stinky" bit you're referring to is the place where we invoke and process all pending effects that have been scheduled by various hooks. In our case component._renderCallbacks contains not just effects for hooks, but also callbacks for class lifecycle methods like componentDidMount.

You may be wondering why we mix those and the reason is simply to save some bytes. And yes, this allows hooks to be used in class components! We don't really advertise that though 👍

@stylemistake
Copy link
Contributor

@stylemistake stylemistake commented Feb 29, 2020

And yes, this allows hooks to be used in class components!

Epic!

@stylemistake
Copy link
Contributor

@stylemistake stylemistake commented Feb 29, 2020

@marvinhagemeister to be clear, I don't think that code is bad in context of Preact, it's just written in a way that makes it less portable, at least that's how it appears. Maybe you may have a suggestion how to unwrap that functionality, or point out some specifics which may be useful to @BaibhaVatsa.

@marvinhagemeister
Copy link
Collaborator

@marvinhagemeister marvinhagemeister commented Mar 1, 2020

Yeah, the code was specifically written for Preact and we never intended it to be used with other frameworks. I doubt that we'll change that as it would likely result in negative effects on our byte count. Some of us are experimenting with bringing hooks into the core itself, thus making it likely even less portable.

You're best bet is probably to copy that code and replace the integration to what's best for inferno. Our test suite should be pretty portable as we only assert against the rendered DOM.

@chrisdavies
Copy link

@chrisdavies chrisdavies commented Jun 19, 2020

I built something similar to hooks for a standalone library I built recently. It was pretty handy, so I decided to bundle up the hooks functionality into its own library xferno. It's not quite React hooks, as it requires you to wrap your components in an xferno function call:

import { xferno, useState } from 'xferno';
 
const Counter = xferno(() => {
  const [count, setCount] = useState(0);
 
  return (
    <button onClick={() => setCount(count + 1)}>{count}</button>
  );
});

@chrisdavies
Copy link

@chrisdavies chrisdavies commented Jun 21, 2020

OK. I published 0.0.3 of xferno. It now works almost identically to React hooks. You no longer need to wrap your functional components in an xferno call:

import { useState } from 'xferno';
 
function Counter() {
  const [count, setCount] = useState(0);
 
  return (
    <button onClick={() => setCount(count + 1)}>{count}</button>
  );
}

It's got some hackery going on under the hood, but if anyone on the core team likes it enough, I'd be down with renaming it to inferno-hooks or whatever. I didn't want to grab that name without permission.

@stylemistake
Copy link
Contributor

@stylemistake stylemistake commented Jun 21, 2020

poggers
One thing that I'm concerned about, is performance, because you essentially wrap every functional component with another component, regardless of whether the hooks are being used. If you ever get to the point of improving performance, you may want to expose inferno internals through the options object, for low-level access to the renderer. I might do some tests soon on /tg/station to measure the performance impact.

I am very excited!

@chrisdavies
Copy link

@chrisdavies chrisdavies commented Jun 21, 2020

I have the same concern. I put together two (very simple) perf files perf-raw.jsx and perf-hooks.jsx, which I manually use for testing perf between the hooks implementation and the equivalent non-hook implemenation. So far, the performance is equivalent. But I'd definitely appreciate better / more real-world performance measurements.

@stylemistake
Copy link
Contributor

@stylemistake stylemistake commented Jul 11, 2020

@chrisdavies I did some A/B tests on /tg/station (which uses IE11 to render UIs), one with just inferno, and one with xferno. Identical code, no hooks, just to measure what amount of overhead xferno was generating during re-renders.

Screenshot of the interface I was testing. It uses a lot of simple html tags with known structure, inferno is known to optimize that very well, plus a pair of SVG curves.

  • Before xferno: 6ms average (sample size: 76)
  • After xferno: 29.5ms average (sample size: 82)

That's a pretty considerable slowdown.

Screenshot of another interface. In this UI, only high level components were used (such as <Box /> instead of <div>, <Button /> instead of <button />, and so forth).

Xferno was only marginally slower, by around 0.5ms, which is insignificant.

So, perhaps this may give you an idea where this slowdown originates from, even though this is only a surface level test.

@simonjoom
Copy link
Author

@simonjoom simonjoom commented Feb 20, 2021

if @Havunen could have a look on xferno will be great to create inferno-hooks in optimal way.. .
#1453 (comment)

I really want to try hook in my next application, as all code seems to work with hooks now.. (Apollo ..Etc) which it seems to give much better coding practice.

@Jarred-Sumner
Copy link

@Jarred-Sumner Jarred-Sumner commented Feb 24, 2021

I wonder if one could implement hooks with good performance via compiling functional components that use hooks into class components at runtime on the first usage. Since all hooks need to run each render, you have guarantees that even for nested hooks, it should be possible to track all the calls. If you can compile the dynamically created functions into functions on a prototype that aren't re-created each call, it should be faster than React Hooks.

Example:

const TextBox = ({text}) => {
  const isDirty = Inferno.useRef(false);

  const markDirty = Inferno.useCallback(() => {isDirty.current = true;}, [isDirty]);
  return <textarea onChange={markDirty} value={text} />
}

This would compile into something like:

class TextBox extends Inferno.Component {
   constructor(props) {
     super(props);
 
     this.$$ref0 = Inferno.createRef(false);
     this._childProps = {onChange: this.$$cb0, value: this.props.text};
   }

   $$cb0() {
     this.$$ref0.current = true;
   }

   render() {
	 this._childProps.text = this.props.text;
	 this._childProps.$$cb0 = this.$$cb0;
     return createElement(textarea, this._childProps);
   }
}

Probably the hardest part of this approach is correctly associating variable names with hook usages, without bloating the bundle with something like acorn

@stylemistake
Copy link
Contributor

@stylemistake stylemistake commented Feb 24, 2021

The real slowdown in xferno is not caused by the overhead of hooks, but by some small bug that negates child/type flags optimization on vnodes, which should be easy to fix.

@simonjoom
Copy link
Author

@simonjoom simonjoom commented Feb 24, 2021

Well lets start to fix that and create a real inferno-hooks.. if one moderator can have a look...
Unfortunately my qualification not good enough to do that ;)

@ScottAwesome
Copy link

@ScottAwesome ScottAwesome commented May 27, 2021

@stylemistake do you know where that issue is, by approximation?

I'm interested in designing hooks for inferno, but I'm not quite sure where this bug maybe

@stylemistake
Copy link
Contributor

@stylemistake stylemistake commented May 27, 2021

@ScottAwesome I have forgot already, but this is probably the piece that creates the slowdown:

https://github.com/chrisdavies/xferno/blob/master/src/xferno.jsx#L217-L223

VNodeFlags are being forced to ComponentUnknown, because well, the target component is abstracted away by the HookComponent and Inferno is required to go through all possible code paths.

VNodeFlags are normally calculated at compilation time by the babel-inferno-plugin, because it knows the structure of all nodes, and it tries to bake this information into createVNode calls.

HookComponent itself is not that thick, but it might contribute just a little bit, because it is still a wrapper.

@ScottAwesome
Copy link

@ScottAwesome ScottAwesome commented May 28, 2021

@ScottAwesome I have forgot already, but this is probably the piece that creates the slowdown:

https://github.com/chrisdavies/xferno/blob/master/src/xferno.jsx#L217-L223

VNodeFlags are being forced to ComponentUnknown, because well, the target component is abstracted away by the HookComponent and Inferno is required to go through all possible code paths.

VNodeFlags are normally calculated at compilation time by the babel-inferno-plugin, because it knows the structure of all nodes, and it tries to bake this information into createVNode calls.

HookComponent itself is not that thick, but it might contribute just a little bit, because it is still a wrapper.

Thanks for the information, @stylemistake !

So, this is something that really should be optimized in core rather than as an addon library I imagine?

Would that be something that would be acceptable?

I wonder how hard it would be to simply extract some of the lifecycle method logic (since you can use it on function components too) and expose that as hooks in that case. I'd like to do something work with that if we can reach some basic clarification on how we would want that to look, so hooks can be baked into inferno directly

@stylemistake
Copy link
Contributor

@stylemistake stylemistake commented May 28, 2021

So, this is something that really should be optimized in core rather than as an addon library I imagine?
Would that be something that would be acceptable?

I assume that both ways are fairly complex. In case of xferno, a lot of work has already been done, but figuring out how to optimize it might be hard. But if you choose to reimplement everything in core, you will spend more time, but might end up with a better implementation.

I have no preference one way or the other.

I wonder how hard it would be to simply extract some of the lifecycle method logic (since you can use it on function components too) and expose that as hooks in that case.

Here is what I think is the most optimal way of implementing hooks:

  • You need to expose more of the internal lifecycle in the options object.
  • Figure out how to store state in relation to vnodes. You could store it preact-style, or maintain it externally, one of the maints should help you with deciding on that, you can join Discord and ask them.
  • The rest is implemented on top of that easily and modularly.

Check stylemistake/tgstation@d1b1385. I didn't do much in my branch, but I did expose various internal hook points, which might be of interest.

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

No branches or pull requests

9 participants