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

Add support for Hooks #20

Merged
merged 22 commits into from Aug 14, 2022
Merged

Add support for Hooks #20

merged 22 commits into from Aug 14, 2022

Conversation

hamsbrar
Copy link
Member

@hamsbrar hamsbrar commented Aug 5, 2022

Description

This pull request is about adding support for hooks, a feature similar to React Hooks.

I won't go into details, feel free to check react docs on hooks for following along.

Primary goal of this pull request is to provide a standard interface for hooks which users can use to create their own hooks. Of course we'll provide few hooks as an example but our focus is more on facilitating easy yet powerful hook implementations that can be created externally.

Hooks

First things first,

A FunctionWidget/WidgetFunction is a function that returns a widget:

Widget widgetFunction() {
  return Span(
      child: Text('hello world'),
    );
}

runApp(app: widgetFunction(), ...);

Functions as such are stateless and yeild same output everytime they get called.

Hooks can powerup these functions by letting you use state and other features inside functions.

Implementation

I've tried couple of ways:

Method 1(with context + a wrapper widget)

Widget widgetFunction() => HooksWrapperWidget((context) {
  var state = context.useState(0);

  return Span(
    child: Text('You clicked me ${state.value} time!'),
    onClick: (_) => state.value++,
  ); 
});

runApp(app: widgetFunction(), ...);

This is without a doubt, the most reliable and safest way one can use hooks.

While this is safe it suffers from other problems. Here hooks are tightly coupled with the context and this limits them to whatever a particular context is limited to. Users who will be using hooks probably looking for something easy to reuse and extend even if it takes away few guarantees so let's see a little bit unsafe method:

Method 2

Widget widgetFunction() => HookScope(() {
  var state = useState(0);

  return Span(
    child: Text('You clicked me ${state.value} time!'),
    onClick: (_) => state.value++,
  ); 
});

runApp(app: widgetFunction(), ...);

This is what we're going to implement. Here, both HookScope and useState are not tied with each other and can be implemented independently, which basically means,

  • Users can easily implement their own hooks.
  • Users can easily use hooks implemented by the others.

How this works?

Similar to React, we'll rely on the order in which hooks are executed.

This puts few responsibilities on the users:

  1. Users should always wrap their WidgetFunction in HookScope.
  2. Users should not call Hooks inside loops, conditions, or nested functions.
  3. Users should use Hooks at the top level of their functions, before any widget/or early return.

We'll try to throw errors or warn users where we can but just know that these rules applies to React as well(read more).

Progress

I've already got a prototype working. I'll outline a slightly different implementation(that of course can change):

  • Add hook and scope interface
  • Add dispatcher methods
  • Add a standard implementation for scope(HookScope)

Rad Hooks

  • Add useContext
  • Add useNavigator

React Hooks

  • Add useRef
  • Add useState
  • Add useEffect
  • Add useLayoutEffect
  • Add useMemo and useCallback

Benchmark results

Hopefully we won't be making any changes to core/existing-widgets so there will be no need.

@juancastillo0
Copy link

Hi!

This looks awesome, thanks for your work! I sent a PR for package:jaspr schultek/jaspr#27, not sure if it helps for this implementation, but I think is related. In particular, maybe it would be nice to try to accommodate MobX or other framework-wide functionalities into the implementation. The events could be provided more globally and implement Hooks along with other things like MobX's dependency tracking from that infrastructure.

There is more discussion in the issue for the PR, but from looking at the code, maybe not requiring the RenderScope to be localized (perhaps only setting the listeners once at the top) and sending the Element that is being rebuilt or unmounted (for all children, not only for the RenderScope) would remove the need to use the RenderScope and just use Hooks or MobX from every where. Maybe the state for Hooks is only setUp if there are being used within the build method of the element to mitigate performance problems. I try to do that in the PR, by only tracking the current HookCtx but initializing only until a hook is used.

Thanks!

@hamsbrar
Copy link
Member Author

hamsbrar commented Aug 7, 2022

Hi there!

I might be completly wrong here but here's that's what I think.

I see stateful/stateless can implement RenderScope(Scope) and this will indeed allow users to use hooks without RenderScope. At least at some places. For example, this will work just fine:

Widget widgetFun() {
  var state = useState(0);

  return Text('${state.value}', onClick: (_) => state.value++);
}

runApp(app: StatefulWidget(
    ...build() {
      return widgetFunction();
    }
), ...);

I know global events will be bit different but if you think about it in current context, end results of having global events will be somewhat equivalent to having all the other elements implement RenderScope(Scope) not just stateful/stateless widgets. By other elements I mean widgets such as HTML widget that accept concrete instances of widgets instead of builder methods. While this sounds good but I don't think they'll improve the situation in any way.

Consider this example,

Widget widgetFun() {
  var state = useState(0);

  return Text('${state.value}', onClick: (_) => state.value++);
}

runApp(app: widgetFun(), ...);

Here we won't be able to do a re-render. Consider one more,

Widget widgetFun() {
  var state = useState(0);

  return Text('${state.value}', onClick: (_) => state.value++);
}

runApp(app: StatefulWidget(
    ...build() {
      return widgetFunction();
    }
), ...);

// run one more app
runApp(app: widgetFun(), ...);

Here hooks used in second app will cause the first app to re-render. I'm sure we can at least prevent listening to wrong context but I don't think we'll be able to re-render the second app so we'll eventually have to force users to wrap their widgetFunction inside a element that implements Scope and accepts a builder method(not concrete widget) which is why we have RenderScope.

@hamsbrar
Copy link
Member Author

hamsbrar commented Aug 7, 2022

We can consider implementing scope in stateful/stateless widgets so that users can use hooks without RenderScope where possible but I think there are more issues here. Consider a stateful widget containing a large tree, parts of which are conditionally rendered:

runApp(app: StatefulWidget(
  ...build(
    if(isAuthed) {
      return Widget1(
        ...
          child: widgetFun(
            ...
            useState('I will mess the other one');

            return Text('logout');
            ...

    } else {
      return Widget1(
        ...
          child: widgetFun(
            useState('please dont');
          
            return Text('login')

Without RenderScope on each widgetFun(s), hooks-order and semantics will not be preserved, hence breaking the foundational rules. I know it's not something users will do intentially but it can happen very easily. I'm also not saying that RenderScope solves everything but it'll prevent the above case and many others.

@juancastillo0
Copy link

juancastillo0 commented Aug 7, 2022

Yes, I believe I understand and agree with your concerns.

The implementation in the PR always has access to the specific element that is being rendered and that element has the markNeedsRebuild method which can be called the Element needs to be rebuilt by hooks or when a MobX dependency changes. As you mentioned, If we use raw function that do not specify a place within the tree (the Element) that can be re-rendered, then it will only work if they use the hooks correctly only within a Widget that has build method. While I believe most people would understand that and in Flutter one usually uses separate Widgets, I see your point and, of course, whether to require the RenderScope to make it more explicit when one can use hooks is your decision. I think we will also need to provide the Widget that can be used anywhere within the tree (the RenderScope), which in this case would be just the regular Builder since all elements would be tracked.

For the MobX dependency tracking, it is a bit different since it does not matter how people use it. If an Observable is being used within the tree, then MobX will register it as a dependency and rebuild the parent element when that dependency changes. There are no concerns about order or conditional execution that Hooks have. In this case, there would need to be a parent element not just raw functions, but perhaps runApp could set it up, or maybe an ObserverComponent that is scoped to the tree and provides MobX tracking for all its children (like in the PR schultek/jaspr#27 where it provides Hooks and MobX tracking).

Although they could be separate functionalities, they could use the same base infrastructure and, basically, the events that you already have implemented. The PR uses the similar willRebuild, didRebuild and didUnmount callbacks for setting up both, Hooks and MobX. From the issue:

From the testing I have made I think all that is needed are these abilities:

  • Know when the element's build execution starts and ends (willRebuildElement and didRebuildElement).

    • to identify the current element when a hook or observer is used
  • Know when the element disposes (didUnmountElement)

    • to remove the tracking for MobX and dispose of all the hook effects/state created within an element
  • Save state for the element (With the reference to the element from willRebuildElement, one could save state)

    • to know what where the previously used hooks and save the tracked observables
  • Rebuild the element (With the reference to the element, one could execute markNeedsBuild)

    • to let the framework know when a dependency of the build method has changed. In the code example, I was using MobX observables for hook states, so they where being tracked automatically. (And the MobX state was created and disposed using hooks)

Thanks!

@hamsbrar
Copy link
Member Author

hamsbrar commented Aug 7, 2022

Just skimmed through the work that you've done, very impressive!

I'm wondering whether dispatching willRebuild and other calls for all elements is really necessary?

The point I was trying to make earlier was very simple, a widget with build method or simply put a "build method" is a requirement for implementing hooks. The closer a build method is, the more reliable & performant our hooks will be. I think reason behind this is quite simple, a tracker in the render phase cannot be precise about the parent element under which a particular hook has executed as hooks reside in execution phase(before framework even has a chance to render). I believe what you will end up tracking is "build methods" not all elements and a better way to do that would be:

  • Setup tracking in widgets that have build methods(stateful, stateless etc).
  • Or Introduce a different widget specifically built for the purpose(like we've RenderScope here).

And when required, these widgets can be made to emit global events.

@juancastillo0
Copy link

I'm wondering whether dispatching willRebuild and other calls for all elements is really necessary?

Yes, I agree. It should not be necessary, since, as you said, for these particular functionalities, what we want to track is reloadable "build methods".

I personally think that "Setup tracking in widgets that have build methods" build be a great addition. In particular, MobX tracking will allow for the developer to almost not care how to subscribe to dependencies within the widget tree, they would use the observables and they would be tracked automatically, similar to https://www.solidjs.com/ (although I believe they compile the whole thing to make it more granular). And for Hooks, it would be nicer since one would not require nesting and perhaps it would be easier to compose different functionalities (Hooks, MobX and perhaps others within the same build method). In Flutter, for example, that can't be done easily since package:flutter_hooks and package:flutter_mobx both require Elements to be implemented and you can't use MobX tracking and Hooks within the same build method (since they are different elements), you need to nest either with Observer from MobX or HookWidget from Flutter Hooks.

I would understand that you do not want to add something like the global events for build methods. However, if there is something I can do to help please let me know. Perhaps, testing a MobX tracking implementation or something.

Unrelated: I have this repo with bootstrap components, I think it would be interesting to port it to rad (although it needs more testing). At the moment it is using a package:deact fork. You can view the components in this web page and the the code of the deployed page. I was wondering what you thought about components in general. I could make an issue or PR to discuss it in more detail if you prefer.

Thanks!

@hamsbrar
Copy link
Member Author

hamsbrar commented Aug 8, 2022

In Flutter, for example, that can't be done easily since package:flutter_hooks and package:flutter_mobx both require Elements to be implemented

Most of the problems we have in Flutter doesn't exists in Rad. It's true that we provide APIs similar to Flutter(widgets) but those are powered by an enitrely different core that's built from ground up and shares nothing with the Flutter framework.

Flutter doesn't provide anything like scope so authors have to build hooks on top of extended functionality that these authors themselves have to provide(by extending elements). Like you said, this becomes a problem when you want to use two different implementations. This pull request was really about standardizing both Scope and Hook interface so that hooks can be created without extending existing infrastructure.

You'll be able to implement MobX hooks using Hooks API and they'll work as long as you've a Scope interface in one of the ancestors. If you want to omit RenderScope, you can have a top level RenderScope, maybe at runApp and it'll power all your hooks. We've a quite performant renderer but in case you start facing issues, just bring a build method close by wrapping the suspected part of tree with a RenderScope widget.

Whether Stateful/Stateless widgets should also provide Scope for hooks is currently under consideration. In particular I'm concerned about hooks order that can go wrong quite easily.

MobX dependency tracking, it is a bit different... there are no concerns about order or conditional execution that Hooks have.

Please correct me if I'm wrong, MobX is being used for only MobX hooks and if yes what about this case:

var _initial = true;

build() {
  if(_initial) {
    useObs(() => 'initial').value = 'changed';
    
    setState(() => _initial = false);
  } else {
    print(useObs(() => 'initial').value); // <- expecting initial or is it changed
  }
}

Thanks

@juancastillo0
Copy link

Yes, useObs uses observables underneath, however in the example app you can use other observable too and initialize them in any way you want, maybe with a useMemo but also you could instantiate them in a StatefulWidget State initState. For example an ObservableList or a code generated MobX store.

The ObservableComponent listens to all the builds of it's children and tracks their dependencies. The MobX part does not need hooks, If you use any observable it will be tracked. In the example, we could have two ObservableComponents, one for the HookCtx that registers a hook for a specific element and the other for the MobX ReactionImpl that tracks dependencies and rebuilds when they change.

In the snippet, it would be changed since the obs state is saved for the element. And uses the same hooks infrastructure.

I believe putting the RenderScope at the top would work and maybe using others if there are performance issues. Is there a problem with putting the RenderScope at the top if there are const Widgets? In that case the observables I think would not be updated if they are used inside the const widget, since the update is at the root and the child is const so it will not be rebuilt. Maybe a could do some tests.

Thanks!

@hamsbrar
Copy link
Member Author

hamsbrar commented Aug 8, 2022

Is there a problem with putting the RenderScope at the top if there are const Widgets?

Yes, you're right, there will be a problem if we've const Stateful/Stateless/Inherited widgets.

So having single RenderScope is not a solution.

In the snippet, it would be changed since the obs state is saved for the element.

See, what this proves is that the whole concept of hooks is fragile. It's based on mere "function call order" and if users cannot keep this call order constant, things will break. Without any help from framework side, users will have to preserve this order in entire application, which is not practical and a typical user cannot do.

While we're trying to observe(track) lifecycle of all elements only so we can re-render parts that are necessary but this process is also helping users in preserving the call order. When we track build methods we're basically chopping this single chain of function calls into smaller chains, and order of these chains can be preserved independently. This, unfortunately, is not enough and users can easily loose track in build methods that comprises of serveral widget-functions(#20 (comment)).

This is where our RenderScope comes in. RenderScope introduces a build method in the chain, resulting in even more but smaller chains making things easy for users. Having RenderScope on all widget-functions means a user have to think about just the function that they're working in.

To conclude, RenderScope helps both users (prevents them from writing bugs) and the framework (functionality is contained in a single unit). I too want to make RenderScope optional but I don't think by making RenderScope optional(and tracking build methods), we're going to help anyone.

Thanks!

@juancastillo0
Copy link

See, what this proves is that the whole concept of hooks is fragile. It's based on mere "function call order" and if users cannot keep this call order constant, things will break. Without any help from framework side, users will have to preserve this order in entire application, which is not practical and a typical user cannot do.

Yes I think I agree, but it works in React and there are perhaps other ways to mitigate it. For example, in React they have linters that check for correct usage. Maybe something like that could be implemented so that hooks are not used in raw functions or inside conditionals. From what I can see, the only difference is that in React, every function is tracked and there would not be any problem, but in Flutter and Dart, I would say most people write the whole Widget class (and is recommended for other reasons too, performance for example specially if it is const). I think there is a discussion about linting in the flutter_hooks repo.

I believe we could separate the Hooks functionality from the MobX one. MobX do not present much problems since the developer does not need to think of using observables in a specific order or without conditionals. It actually solves them, since it does not require you to move the RenderScope if you move the observable.

I could make some tests or maybe open an issue and help with a PR if you think that the MobX part would be a good addition. We would need the global events willBuild didBuild and willUnmount.

Thanks again and I am sorry if it is becoming too long of a discussion, I just want to help and I believe this would make it easier to use. The MobX part would be a game changer I think, there would be no need for context.watch, context.listen or setState, each observable register itself to update a Widget when in changes.

@hamsbrar
Copy link
Member Author

hamsbrar commented Aug 8, 2022

Yes I think I agree, but it works in React and there are perhaps other ways to mitigate it.

I don't know the exact details but I think it's because:

  1. They are using JS.
  2. They have a optional build step for JSX which hides away lot of details.
var functionComponent = function() {
  var {state, setState} = useState('initial state');

  return <div>{state}</div>;
}

ReactDOM.render(
  <functionComponent />, ...
);

Becomes:

var functionComponent = function() {
    var state = React.useState('initial state');

    return React.createElement('div', {...});
}

ReactDOM.render(
  React.createElement(functionComponent), ...
);

Notice, they're passing a callback to createElement which allows their renderer to execute hooks during render(where they'll be able to precisely tell parent element). Of course there are plenty of other ways in JS so they might be doing something else as well but at least we can see that they're delaying the execution of hooks. This is something we can do in Dart if there's support for unions:

Widget widgetFun() => Text(useState('hello world').value;

runApp(app: widgetFun, ...) // -> app:  Widget | Widget Function() 

I have this repo with bootstrap components I think it would be interesting to port it to rad

I think a components library will help. As long as we can maintain it, we can have it here too :)

The MobX part would be a game changer I think. I could make some tests or maybe open an issue and help with a PR if you think that the MobX part would be a good addition.

Alright. I'll take a deep look at MobX and the PR(that you linked the other day) and I'll let you know.

Thanks again, for your time & discussion. I appreciate it!

There are plenty of ways a external scope implementation can
abuse dispatcher. Setting scope and not unsetting it (or
repeatedly setting it) is the easiest one that I think this
change will prevent(if we ever decide to make
scopes public).
@hamsbrar hamsbrar marked this pull request as ready for review August 9, 2022 04:04
@hamsbrar hamsbrar merged commit e6f3eec into main Aug 14, 2022
@hamsbrar hamsbrar deleted the dev-hooks branch August 14, 2022 04:42
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

Successfully merging this pull request may close these issues.

None yet

2 participants