zouloux / prehook-proof-of-concept

This is a proof of concept of a React Hook like implementation for Preact.
MIT License
33 stars 2 forks source link

Thoughts #1

Open danielkcz opened 5 years ago

danielkcz commented 5 years ago

From all those proposals from RFC this one definitely looks the tidiest :) I am curious how you would approach custom hooks? In my opinion, these are what makes React hooks the most brilliant - ability to separate and share encapsulated behaviors.

mcjazzyfunky commented 5 years ago

When I've opend #2 I have not seen that @FredyC has already opened this issue. Maybe it's better to ask my question here:

When using the current React hook API you often have custom hooks where you pass values, like useSomeCustomHook(a, b, c). Let's say in this example a, b and c are strings depending on different props values. Using your proposal a, b and c would have to be getter functions. Don't you think that makes things kind of complicated? For example: useSomeOtherCustomHook(() => props().title) That's currently my main concern about using a factory pattern (not only in your proposal).

[Edit: To be a bit clearer what I mean: With a factory pattern, hooks (including custom hooks) will only be run once per component instance, when the component is mounted - therefore you'll have to handle with a lot of getter functions instead of using the values directly]

AlexGalays commented 5 years ago

Yes, the fact that the README doesn't mention custom hooks is worrying. Most of the hooks I use are actually custom.

zouloux commented 5 years ago

Hi all, thanks for your reviews ! I'll check implementation of custom hooks. For now every state or props are functions so it should work, but I need to check more on this to catch any issue.

function ComponentFactoryScope ( props )
{
    const myState = useState( 0 );
    useCustomHook( props('color'), myState );
}
[...]
function useCustomHook (color, state)
{
    color(); // -> 'blue'
    state(); // -> 0
    [...]
    useState( ... ) // should works
    useEffect( ... ) // should also works here
}

props('color') returns a function to get color when called. So named props in your component does not impose naming in your custom hook, which is the main thing worrying about this implementation.

I hope this is clear, I'll catch up here soon :) Thanks !

mcjazzyfunky commented 5 years ago

Please allow me to add a different example to show that with a API based on a factory pattern you have to deal with getter functions almost all the time instead of using normal values as it would be possible with the current React hook API:

// current React hook API (width and height are numbers)
const [width, height] = useWindowSize()
useFancyHook(`The window size is ${width}x${height}`)

// some API based on factory pattern (width and height are functions)
const [width, height] = useWindowSize()
useFancyHook(() => `The window size is ${width()}x${height()}`)
danielkcz commented 5 years ago

@zouloux Isn't there going to be a performance hit creating a get/set function for each prop value on every change? Also what about deep props? If you want to pass a single value from some complex structure, how would you create such a function?

zouloux commented 5 years ago

Quick reply : I think with this implementation, the only solution is to think with the "Factory returns render" pattern all the way. It needs more reflexion but I see the problem of always passing function for all values here. I'll do some research on this to avoid cumbersome pattern. The goal is to avoid the need of useMemo and useCallback so maybe there is no better solution. I'll be happy to read from anybody if you have an idea :)

danielkcz commented 5 years ago

One way could be using eg. mobx.observable which can wrap around an object and provide easy tracking of changes. (P)React could make their own implementation optimized for such use case, but it's a nice API interface in general imo.

PutziSan commented 5 years ago

Hi, I really like the idea to use hooks with a factory-function. I find especially your suggestion interesting that props is a function to always get the current props values in effects. I also think the proposal is better than the one I made before (PutziSan/react-factory-hooks).

I've incorporated this idea into mine and revised it (I hope it's okay). My proposal now differs in that I would still add props as a parameter in the render function. I have described the reasons for this in the readme under Why the render function should get props as argument. Feel free to share your thoughts about this.

danielkcz commented 5 years ago

@PutziSan @zouloux

I think you are both forgetting one important detail ... default props and I don't mean that hack-ish static defaultProps property, but real default values you can define when destructuring props in a component scope. It means much better colocation and it's clear what default value is for a particular prop.

Also things like props('color') are much harder to much to tackle with static typing (eg. TypeScript). Especially if you would want to go extra length with deep objects and some dot notation. I really don't recommend this.

zouloux commented 5 years ago

@FredyC I agree with the prop('color') way of doing it which is not very "struct proof", but Typescript definitions are made (kind of) in the repo and autocompletion + errors are working when you enter a wrong string (thanks to keyof TS keyword).

danielkcz commented 5 years ago

Ok sure, that was mostly just aside note :) I am more concerned about those default props really.

PutziSan commented 5 years ago

@FredyC

I think you are both forgetting one important detail ... default props and I don't mean that hack-ish static defaultProps property, but real default values you can define when destructuring props in a component scope. It means much better colocation and it's clear what default value is for a particular prop.

You're absolutely right. And for me that was also the reason to reject the idea of getProps as a parameter for the factory function. I've just published a third draft with a new documentation (PutziSan/react-factory-hooks), if you like you can come back and share your thoughts/feedback like everyone else.

Especially the part "Why the factory-pattern if the current react-proposal is so popular?" in the FAQ might be interesting and I would be interested to know what the opinions are.

zouloux commented 5 years ago

Hi guys, I updated prehook lib and examples with some useProps and custom hooks example. -> https://github.com/solid-js/prehook-proof-of-concept I know this custom hook example is the simplest (no arguments) but I try to do it step by step. States and Props are pretty good and usable like this to me, no more mandatory states() call to read values (I host current value into function) so it may be performant. Function is still callable so useEffect can just call it to know if it changes.

I know this (factory pattern implementation of hooks) is doomed since everybody will go to the React's way, but I found it interesting to search a bit following this track, this feels pretty right !

I read your comments here, there and there, and a lot of really interesting thinking is going on :)

zouloux commented 5 years ago

@AlexGalays Do you have any good example of real life custom hook ? It would be very useful to know how it works. Thanks !

AlexGalays commented 5 years ago

One I wrote recently:

export default function useIsFirstRender() {
  const isFirstRender = useRef(true)
  useEffect(() => { isFirstRender.current = false })
  return isFirstRender.current
}

or all the hooks on https://usehooks.com/ !

mcjazzyfunky commented 5 years ago

[Edit (much later) - Just for the record and fyi: As to be seen in the comments below, there has been a misunderstanding on my side - when I've written this comment - about the usage of useProps, so parts of the following concerns may sound a bit "weird"]

@zouloux
First of all: Your nice proof of concept here is not nearly "doomed" (as you have written above), the ideas you've presented are very interesting and sharing ideas is always very important, you never know whether someone will read your POC one day in future and will take some of your ideas and mix it with her/his own ideas and will use those combined ideas in some other project. So again many thanks to you and all the others who present their ideas :)

May I asked you kindly whether you still think that this "useProps(...)" hook is really a good idea (same question to @spion who has preseted a very proposal somewhere in that large React hooks RFC thread)?

My main concern is that this useProps does not really support good typing in TypeScript and Flow. Moreover if you want to use some props dependent value inside of a custom hook then you will have to use that "useProps" inside of the custom hook which would make the custom hook dependent of a given props data structure (or at least a partial of it), which is something really suboptimal (of course you could write a helper custom hook like "const getXXX = useMappedPropsGetter(props => props.xxx)" or something similar to get a getter function to be passed as argument to the custom hook, but that would not really make things a lot better).

In your first draft the pattern was more like the following (actually I liked that proposal) - I do not see any problems with default values there (nevertheless it's true that the usage of props/prop default values is not really (quote) "colocated"):

export default prehook(function Counter(props) {
  const
     { initialValue = 0 } = props(),
     count = useState(initialValue)

  return () => {
    const { label = 'Counter', onChange } = props()
    ...
  }
})

You've introduced that "useProps" hook as a reaction to the first sentence of this comment https://github.com/solid-js/prehook-proof-of-concept/issues/1#issuecomment-440332118 But I think you neither gain (quote) "much better colocation" nor a better handling for default prop values (at least for non-trivial components).

The following is my very personal opinion - no problem if other opinions differ completely, but if you do not have any problems with providing component meta information very explicitly (similar to Vue and some other UI libraries) and you do not have any problems that this meta information will not be strongly minified (one advantage is that this meta information could be available 100% at runtime) then you could also do something like the following, so you'll have enough (quote) "colocation" regarding props, prop types and prop defaults, IMHO:


export default component({
  displayName: 'Counter',

  properties: {
    initialValue: {
      type: Number,
      validate: Spec.integer, // 3rd party validation library
      defaultValue: 0
    },

    label: {
      type: String,
      defaultValue: 'Counter'
    },

    onChange: {
      type: Function
    }
  },

  init(props) {
    const
      { initialValue } = props(),
      count = useState(initialValue)

    return () {
      const { label, onChange } = props()
      // ....
    }  
  }
})

// BTW: See this "component" function as overloaded:
// It also accepts a "render" function instead of that
// "init" function for simple function components.
// I'm quite sure most folks will not like this
// "component" function especially because
// it's a bit verbose - but anyway....
spion commented 5 years ago

useProps is one example, another is simply to have the event argument in the callback. At the bottom of that thread I added examples where you can access the props through the event argument.

I don't think react hooks have been designed / thought out very well. I have some worries about the soundness of the design, although not many, but even more worries about how alien they really feel to the language.

zouloux commented 5 years ago

Thanks @mcjazzyfunky ! useProps here is an Higher Order Function to allow props changes reading in custom hooks. To me, custom hooks should never talk about "props", because props are from component and named for component. Custom hooks should just get a list of arguments, like a normal function. This is why useProps HOF can convert to function which return prop value when called :

const props = useProps({
    title: 'Default title'
});
useCustomHook( props('title') );

Here useCustomHook can call first argument to get current title value, in a useEffect for example.

function useCustomHook ( getTitle )
{
    useEffect( () => document.title = getTitle() );
}

There are some examples in this very repo : https://github.com/solid-js/prehook-proof-of-concept/blob/master/src/HookedComponent.tsx#L205

For the static Typing, everything seems to work with this implementation, since useProps() never goes to custom hooks and is kept inside component.

https://github.com/solid-js/prehook-proof-of-concept/blob/master/lib/prehook/prehook.ts#L240.

So to be honest I don't know if the useProps is a good idea, I'm not even sure if the factory pattern is haha ! But to go fully into the factory pattern concept, I think everything should go towards HOF. Not a single reference should be made to a non function objet, and it seems to work pretty well. This is why useProps seems to get along with this concept.

Now, the performance and complexity issues were the main concerns about the React implementation, and I don't really know if factory + HOF everywhere is better than the React's pure functions implementation. I don't know either which one is the better following functional programming rules.

But I'm happy that everybody can think about it and try some ideas, maybe it can be useful in some cases.

zouloux commented 5 years ago

@mcjazzyfunky for the "type" part, I'm more of a Typescript guy, so every meta are not pushed to bundles but this is just a different way of thinking. Here types, are handled at compile time :

const props = useProps({
    title: 'Default title'
});
useCustomHook( props('title2') ); // This fails to compile since 'title2' is not a key of `GProps`
mcjazzyfunky commented 5 years ago

@zouloux Oooh, my bad! Seems I have not read your examples carefully enough. Thanks for your explanation. That means you will use useProps exactly one time per component (at least in case your component has any properties) and useProps will never be called inside of a custom hook. Nevertheless type inference with "useProps" will still be a minor issue in case a property has no default value or the default value is null (as you have shown in your comment https://github.com/solid-js/prehook-proof-of-concept/issues/1#issuecomment-448939521). But as you are only using "useProps" at the most once per component and the type/interface of the component's props will normally be declared explicitly (at least for non-trivial components) it should not be a big deal to write const props = useProps<IProps>({ ... }).

I'm not even sure if the factory pattern is [a good idea] haha

Hehe, that's the big question ... I think we still do not have the answer ;-) As written somewhere above I really do not like all those getter functions (or similar stuff), but I do not really know if that's really a showstopper, anyway... I'm fine with React's hook proposal but I would really love to see a real good and equally powerful alternative API to it (independent whether someone would actually use that alternative API).

@spion Thanks for your answer - I think lacking proper type inference may also be an issue with that event.target proposal ... mmh, or maybe I am just missing something ;-)

mcjazzyfunky commented 5 years ago

@zouloux May I ask you something completely different (I really would love hear to your opinion abot the following)? Just assume that it's 2022 and you have too much spare time, so you decide to write your own tiny little JS/TS UI library (as we all know that there are not enough JavaScript UI libraries out there yet :wink:). Please also assume that in 2022 TypeScript will have a great support for type inference with generator functions (in 2018 unfortunatelly yield whatever has always the type any, but in 2022 this issue has been fixed). You decide that your new UI library will only support EcmaScript>=2018 (be aware that you can easily distinguish "normal" functions from generator functions then).

In almost all hook API alternatives "useState", "useEffect" etc. are global effectful functions - and I'm quite sure we all agree that global effectful functions are not really the coolest things in universe. Moreover most of them have to be an essential part of the core of the library.

Now my question: In this very hypothetical 2022 scenario would you still stay with your API as is or would you switch to a slightly different generator based API like follows?


// Nothing special here
function SayHello({ name }) {
  return <div>Hello, {name}</div>
}

// Using generators for effectful components
function* Counter() {
  const count = yield useState(0)

  function handleClick() {
    count(count() + 1)
  }

  return () =>
    <button onClick={handleClick}>
       {count()}
    <button>
}

Base hooks like useState, useEffect etc. are pure functions that only return certain data structures and are only simple helper functions not really necessary in core (yet very, very helpful of course).

For example:

export function useState(value) {
  return {
    type: 'useState',
    value
  }
}

Custom hooks would have to be implemented as generator functions.

The main disadvantages are:

The main advantage is:

Please ignore testability in your answer - this will surely not be a big deal in both variants.

zouloux commented 5 years ago

It's fun to imagine it in 2022 and yeah why not !

I'm not comfortable enough with generators to help you on this, I see quickly how it works but it's not a revolution to me /for now/ because i have not fully understood and tried them. But with this idea, if I get it well, the framework part will next() the component's generator until all hooks are retrieved so hooks will be essentially pures, that's right ?

It could work well and hooks will be more light so more responsive maybe. The only caveat for me is that, and you pointed it, all hooks will work thanks to their call order (like React's implementation) so no hooks in conditions and a bit of magic happening under the hood.

Also maybe there will be more UI libs in 2022 than people and I'm pretty sure creating a UI lib from scratch will be learned at school haha

spion commented 5 years ago

@mcjazzyfunky you are not missing anything - indeed proper type inference is difficult. Which is why I also think the dispatcher should be explicit i.e.

this.useProps() or useProps(this)

Same goes for the event based hooks:

useListener(this, 'after-paint', e => {
    // dont actually replace the subscription if props point to the same userId
    if (subscription.userId == e.target.props.userId) return; 
    // ... more code ...
});

That way this is Component<Props> and useProps can have a return type Props taken from there. In typescript this is supported:


// not an actual argument, just typescript's way of specifying the `this` type
function MyComponent(this: Component<Props>) {
  let props = this.useProps() // returns Props
  this.useEffect(...) // use props() within the effect
  return props => <div>Hello {props.name}</div>
}

And we arrive at a a zero-magic hooks implementation at the cost of (very) little extra noise.

Another gain from the explicit dispatcher is that if we use this.hookName(args) for the built-in hooks, and we dispatch JSX via this as well, we get 100% framework independent components. As long as the this argument passed by the framework supports the interface the component requires, the component can be used with that framework. Same goes for the custom hooks - they would also work with any framework.

Neither the component nor the hook will have any need to actually import react, preact, or any specific runtime dependencies. For typed languages, they would only need to import types and interfaces describing the general framework API.

spion commented 5 years ago

P.S. I don't deny that this in JS is confusing - but at least its shareable knowledge.

But hooks are also equally confusing, if not worse. Will the closures I pass to useEffect be updated to reference the new props, or will they be used only once? What about the argument passed to useState, will it update the state if the argument changes? The answers to these questions are completely unintuitive and impossible to know without looking up the documentation. Its just not clear what runs when.

However, at least you can learn how this behaves, and transfer that knowledge everywhere you use JS. Learning hooks semantics is not nearly as broadly applicable.

Also, the real solution is to lobby TC39 to help finally fix the this learning problem by introducing the bind operator

edit: P.P.S. I don't understand the compiler-related parts of the motivation document, but most of the other stuff that hooks solve could have been solved simply by adding this.addListener for the lifecycle methods, as well as this.addState

mcjazzyfunky commented 5 years ago

@zouloux

The only caveat for me is that, and you pointed it, all hooks will work thanks to their call order (like React's implementation) so no hooks in conditions and a bit of magic happening under the hood.

Oh, actually the "call order" does not really matter (in difference to React), as those calls are only be done once at the component's initialization phase (same as in your proposal). But anyway, the whole thing was just a hypothetic idea 😁 ...

@spion

Thanks for your feedback.

Actually the following three things you've written somewhere in your comments really catched my interest:

Let's just assume for one moment that there's conceptionally not much difference between this.useWhatever(0) and useWhatever(self, 0). This would eliminate issues regarding this and the binding operator ::. Also let's assume that passing that self argument at each invocation of useXXX(self, ...) is not an inacceptable syntactical overhead.

This would follow to something like:

function Counter(self) {
  const [getCount, setCount] = useState(self, 0)

  function handleClick() {
    setCount(getCount() + 1)
  }

  return () =>
    <button onClick={handleClick}>
      {getCount()}
    <button>
})

Doesn't really look like a new concept, but actually it may be: There are no hook functions any longer, useState and all other use*(...) functions are completetly written in userland and basically as "normal" as any other function (but they are always non-pure). While the React hooks proposal is conceptionally based on fancy new stuff like "Algebraic effects with handlers" etc. the above example actually only uses boring old-school design patterns from the 90ies 😃 ... That self argument is of the following type ... or at least something similar ... with those "onXXX" methods mutiple lifecycle event handlers can be registered (the return value is the unregister function):

interface Component {
  forceUpdate(),
  onDidMount(handler: (self: Component) => void): (() => void),
  onDidUpdate(handler: (self: Component) => void): (() => void),
  onWillUnmount(handler: (self: Component) => void): (() => void),
  onDidChangeContext(ctx: Context<any>, handler: (self: Component) => void): (() => void),
  // ... plus some further ....
}

I think that the interface Component should not be generic and independent of properties. Not really sure yet how to handle props (an additional argument for the factory called getProps would be one possibility, but maybe there are better solutions for that, anyway...)

Of course this is just a initial incomplete draft for this alternative API. Any opinions about such a solution?

mcjazzyfunky commented 5 years ago

Not really sure, but maybe this is better than my previous suggestion above:

type Listener = () => void
type Unsubscribe = () => void

// type of the "self" object
interface Component<IProps = {}, IMethods = {}> {
  getProps(): IProps,
  forceUpdate(),
  onDidMount(listener: Listener): Unsubscribe,
  onDidUpdate(listener: Listener): Unsubscribe,
  onWillUnmount(listener: Listener): Unsubscribe,
  onDidChangeContext(ctx: Context<any>, listener: Listener): Unsubscribe,
  setMethodsHandler(handler: IMethods): void, // counterpart to useImperativeMethods
  // ... plus some more ....
}
spion commented 5 years ago

@mcjazzyfunky thats pretty much it, yeah. I would also add createElement to the Component interface so JSX can compile to self.createElement for zero dependencies on the framework. In that sense it may be easier to just use this, because its guaranteed to be syntactically the same (you don't have to look up the name of the first argument of the component function)

I'm still reading up on algebraic effects, and so far they seem pretty awesome. I'm not sure that a dispatcher global variable and a persistent call index are a good way to implement them. I would be more comfortable with generators, or something like a monadic interpreter i.e. https://github.com/natefaubion/purescript-run

This also looks interesting: https://github.com/spicydonuts/purescript-react-basic/blob/hooks/examples/refs/src/Refs.purs#L21 (still digesting how its done though)

spion commented 5 years ago

Not really sure yet how to handle props (an additional argument for the factory called getProps would be one possibility, but maybe there are better solutions for that, anyway...)

I missed this part. You could pass the initialProps to the factory.

The nice thing about needing the current props, you would always need them from within a callback due to the separation of initialization and render phase. Therefore we can just change the type of Listener:

type Event<T> = {
  props: T
  // other aspects of the component here...
}

type Listener<T> = (e:Event<T>) => void

type Component<IProps = {}, IMethods = {}> = {
  props: IProps;
  forceUpdate(),
  onDidMount(listener: Listener<IProps>): Unsubscribe,
  onDidUpdate(listener: Listener<IProps>): Unsubscribe,
  onWillUnmount(listener: Listener<IProps>): Unsubscribe,
  // ... plus some more ....
}

Similar to DOM events, through this event you could have access to the "event target"'s props via e.props.someProp

Here is useEffect implemented as a custom hook. or rather, useConditionalEffect

function useConditionalEffect<T>(self: Component<T>, 
    condition:(oldProps: T, props:T) => boolean, 
    f:(e:Event<T>) => void) {
  let cleanup = null;
  let oldProps = null;
  self.onDidUpdate(this, e => {
    if (!condition(oldProps, e.props)) return null;
    oldProps = e.props;
    if (cleanup) cleanup(e);
    cleanup = f(e);
  });
  self.onWillUnmount(e => cleanup && cleanup(e))
}

Thanks to the self argument, the types align well.

p.s. Nothing stops you from adding hooks that work in the render phase too i.e. this.onAfterThisUpdate could run once after the current render() is applied to the component. That way you can have easy access to the current props captured in a closure. (Persistent call index is still not necessary unless you want to replace things from a previous invocation)

p.p.s. Here is an easier useProps:

let props = () => this.props;

or for a specific prop:

let currentId = () => this.props.id
mcjazzyfunky commented 5 years ago

TL;DR: I like the idea of using a factory pattern to implement stateful/effectful components ... but yet, after all, I prefer the original hook API by the React team for pragmatic reasons. Nevertheless my personal preference how such a factory pattern based API should look like is the following:

http://jsfiddle.net/6Lox98yf/


The last couple of days I've played around with some of the ideas discussed in this thread and I would like to share my experiences with you regarding this (hoping not to bore you too much :wink:). The following is highly biased and just my very personal opinion and it's of course completely fine if your opinion differs :smile: ... anyway:

  1. Regarding this const [getCount, setCount] = useState(0) vs. const count = useState(0) // "count" is a getter/setter hybrid

    I've changed my mind completely here. Now, I really prefer the [getCount, setCount] solution, because otherwise you never know directly whether a given something variable is a "normal" value or a getter or even a getter/setter hybrid - this feels quite annoying with non-trivial components.

  2. Regarding this self object of type Component<P, M> that I have mentioned above: I think it's way better just to call it c as it will be used quite often => simply less optical noise.

  3. Regarding that above mentioned default props issue: I really believe that declaring the defaultProps declaratively is not (quote) "hack-ish" at all. If you like to pass the current props (and also maybe the previous props) around in event callbacks or as input arguments in functions like for example in @spion's useConditionalEffect example above, a declarative defaultProps configuration is a reasonable and legitimate way. Of yours this is not absolutely necessary - introducing @zouloux's above mentioned useProps(defaultProps) hook may also do the job (whatever the return value will be exactly), but I think using just props or c.prop whenever you need it is far more intuitive.

  4. Regarding the question whether the introduction of such an above mentioned c object (formerly known as self) makes sense or not is surely highly a matter of taste. Nevertheless I would really prefer it as then you have 0% magic and 100% type safety and handling the current and previous props is much easier.
    BTW: @localvoid's "ivi" project is doing something similar: https://github.com/localvoid/ivi#components (but c is an opaque object there).

  5. Regarding the concrete type of the c (fka self) object (see interface Component above): I think that should be basically as low level as possible - if you want to use a higher level API just use the "use*" functions. So I would not really suggest to pass props as argument for the listeners, as it is not necessary. But like everything, that may be just a matter of taste. Maybe replacing onDidMount, onDidUpdate, onWillUnmount (in the Component<P, M> interface) by one single c.handleEffect(...) method (similar to "useEffect") could be nice (not really sure - anyway, these are just details). Using c.props (like @spion has suggested) instead of using c.getProps() (like I have suggested initially) is surely the better solution (maybe adding c.prevProps as sugar for convenience may be an option to think about).

... however (as mentioned above) I prefer the React team's hook API to a factory based API


[Edit: 2018-03-31] Removed stupid idea....

[Edit: 2019-03-13] "toCounsumer" , which btw returns a function that returns a function

const Counter = component('Counter', c => {
  const
    getProps = useProps(c, { label: 'Counter', initialCount: 0 }), // handling defaults
    [getCount, setCount] = useState(c, getProps().initialCount),
    getPrevCount = usePrevious(c, getCount),
    consume = toConsumer(getProps, getCount, getPrevCount) 

  // this could be a downer for smart compilers and linters,
  // and it is obviously a bit verbose
  useEffect(c, consume((props, count, prevCount) => {
    // ..
  }))

  return consume((props, count, prevCount) => {
    return <div>...</div>
  })
})
mcjazzyfunky commented 5 years ago

@mindplay-dk As you have asked on different platforms (e.g. https://twitter.com/mindplaydk/status/1104730608574566400) whether your proposal https://codesandbox.io/s/z6x4vkx2rp has already been discussed somewhee: First, thanks for sharing your ideas. Maybe you are interested in this discussion thread here, where similar ideas have been discussed (it's about the concept, not that much about details). My latest example right above this comment (labeled with "[Edit: 2019-03-13 ...]"), is quite similar to your proposal, I think (just don't get confused with that consume helper).

Here's an interesting comment by Dan regarding some downsides of such factory patterns for component initialization: https://github.com/reactjs/rfcs/pull/68#issuecomment-455189646

To summarize some issues regarding the factory approach:

[Edit - some hours later: After having read localvoid's great comment directly below, most of the following "Issues" aren't "issues" any longer if you follow localvoid's proposal to create dataflow pipelines in the outer function and wire and feed them in the inner function - really a smart idea, thanks Boris]

localvoid commented 5 years ago

@mcjazzyfunky outer function should create dataflow pipelines and inner function should wire dataflow pipelines and pass input data.

const initState(initialState) => ({ count: initialState });

function reducer(state, action) {
  switch (action.type) {
    case 'increment':
      return {count: state.count + 1};
    case 'decrement':
      return {count: state.count - 1};
    default:
      throw new Error();
  }
}

const Counter = component((c) => {
  const [getState, dispatch] = useReducer(c, reducer, initState);
  const onIncrement = () => { dispatch({type: 'increment'}); };
  const onDecrement = () => { dispatch({type: 'decrement'}); };
  const updateDocumentTitle = useEffect(c, (title) => {
    document.title = title;
  });
  return ({ initialState }) => {
    const state = getState(initialState);
    updateDocumentTitle(`Title: ${state.count}`);

    return (
      <>
        Count: {state.count}
        <button onClick={onIncrement}>+</button>
        <button onClick={onDecrement}>-</button>
      </>
    )
  };
}

Yes, it is slightly more verbose than react API in some use cases and less verbose in use cases like conditional memoization.

mcjazzyfunky commented 5 years ago

@timkendrick Thank you very much for your comment here https://github.com/reactjs/rfcs/pull/68#issuecomment-476917622 and also many thanks for your examples here https://codepen.io/timkendrick/full/wObdyb

Please allow me to answer in this nice little thread here and not in the one you have originally posted (actually the mood in that React RFC thread is far too emotional in my opinion, that's why I prefer this thread here). Your examples are a great base for disussion. Hope you do not mind if I concentrate on some IMHO downsides now.

Especially in your example "useMemo" you see that your interpretation has still those often mentioned issues:

I personally think the approach that @localvoid has presented above is really interesting, as it has non of those issues.

Here's a litte re-write of your "useMemo" example in some kind of @localvoid's style .... at least if my interpretation of his ideas is correct. Please ignore that this example is based on some strange experimental UI API - this is just a fun project of mine and the its code base is currently embarrassingly ugly and buggy - so please do not expect it to work properly apart of this little demo. And please just ignore that this c argument is been passed around (it's necessary for the underlying library)

https://stackblitz.com/edit/react-akwaf3?file=index.js

I'm not claiming that X is better than Y ... just saying that some things could still need some improvement... and that those things are worth a discussion, I think....

timkendrick commented 5 years ago

I'm not claiming that X is better than Y

I personally agree that your version is nicer (it's definitely more pure), however it seems that sadly our criteria for what makes a good API differ from those of the React team.

After all my previous suggestions for purer APIs were rejected I was attempting to propose an alternative that was as close to the current implementation as I could.

I'm really not a fan of the current API, I was just trying to find a halfway-acceptable compromise that deviates as little as possible from the existing implementation so people had as little to complain about as possible.

So in short: I agree with you – however I suspect we've got zero chance of convincing the React team :)

mcjazzyfunky commented 5 years ago

@timkendrick Oh, actually my intention was never to "convince the React team" or anybody else. I'm just extremely fascinated by the question what could be real good ways and patterns to describe/implement components, especially which alternatives are there, and the best way to get answers for that is to discuss that topic with others ... and you and others have shared some really interesting ideas, and I am really thankful for that (and surely a lot of other readers are equally thankful). That "factory pattern" is just a promising approach IMHO ... not more, not less ....

Obviously, I use React's hook API for my components now and I have a lot of fun with it (just million times better than the old class API - to exaggerate a little bit :smiley:). Of course the React team will not change the current hook API unless someone will come up with some better idea and let's be honest, there's really no proof at all that whatever variant of the that factory pattern is really better than React's current hook API (most things seem to be more like a matter of taste, I think).

Since Version 16.8 all non-legacy React component's are represented by "normal" (yet possibly stateful, of course) unary functions. This fact alone is really, really amazing. Who would have thought a few years (maybe even months) ago that this simplicity and unification would ever be possible?

brucou commented 5 years ago

@mcjazzyfunky

Since Version 16.8 all non-legacy React component's are represented by "normal" (yet possibly stateful, of course) unary functions. This fact alone is really, really amazing. Who would have thought a few years (maybe even months) ago that this simplicity and unification would ever be possible?

The perceived simplicity is in the syntax (the thing too many developers like to rave about). It is not so clear that the concept is simpler than alternatives. At the end of the day programming is about concepts, not syntax; and the costliest mistakes to correct are the conceptual ones. I actually find ivi-style hooks simpler. In addition, the author mentions:

There are several differences in the ivi API because we don't need to support concurrent rendering, and because of it we could try to solve some flaws in the React hooks API design:

  • Hooks rules
  • Excessive memory allocations each time component is updated
  • "Memory leaking" caused by closure context sharing

That said, the ability to package and reuse functional units of hooks is real, whatever the approach. Time will tell which approach is best. I personally plan to stay away from React-hook for at least 9 months, to get the perspective that has been acquired by early-adopters.

In the meanwhile, in some of these proposals that are compatible with React <16.8 could be packaged into a library, I would on the opposite be an early adopter of them and give feedback in the coming months. There is nothing wrong with React picking up the best API for themselves - they serve React's interest first and foremost. But I would welcome alternatives.

mcjazzyfunky commented 5 years ago

@localvoid

May I ask a question?

In your example above there's a pattern like the following:

const [getState, dispatch] = useReducer(c, reducer, initState)
const onWhatever = () => { dispatch({ type: 'whatever' }) }

Let's assume in that callback function the dispatched action object needs some current state value as payload:

dispatch({ type: 'whatever', payload: state.someValue })

How do you handle such use cases normally?

Something like the following? const getOnWhatever = memoize(someValue => () => { dispatch({ type: 'whatever', payload: someValue }) }) or const getOnWhatever = useCallback(c, someValue => () => { dispatch({ type: 'whatever', payload: someValue }) }) or do you have some better solution for such cases?

Thanks a lot in advance.

localvoid commented 5 years ago

Depends on how frequently component will be updated in the application. When I don't care about update performance, I won't even memoize this callbacks, otherwise it is either your example with memoize(...), or this one:

const C = component((c) => {
  let state;
  const [getState, dispatch] = useReducer(c, reducer, initState);
  const onWhatever = () => { dispatch({ type: 'whatever', payload: state.someValue }) };

  return () => (
    state = getState(),

    <div onClick={onWhatever}>{state.text}</div>
  );
});
mcjazzyfunky commented 5 years ago

Hope you do not mind if I write an additional comment to this topic after all this time, even by running the risk that you guys have already completely lost interest in this stuff here :smile:

Just to be clear in advance: The hook API in React is still (maybe even by far) my preferred way of implementing stateful/effectul components.

Nevertheless I have implemented in a R&D fun project of mine some of the ideas that we all have talked about regarding that factory pattern thingy. For the sake of diversity, I have tried to implement things completely different to React and also quite different to ivi. BTW: That little fun library I am using in the examples below is a bit more than just an additional way of writing hook-based components ... but that's not important here ... just in case you are wondering why the whole API is so "different".

The goal of this fun library is to find a completely magic-free, old-school technique based way of implementing components ~where the render functions shall be pure (the latter in difference to React and ivi).~ [Edit: the latter statement was not accurate]

Like said this is just a fun project and I currently do not plan to work on it any longer. That's why I finally would like to show you four little demos to present the outcome of this litte project. I would really love (if you do not mind) to hear some comments whether this is still way too impractical or whether there may be some sub patterns you perhaps even think might be a bit promising. The main issues are still verbosity and handling with all these "getter functions"/"wrapped changing values". I use a simple utility function called wrapGetters in the last two demos to (hopefully) make that a little less PITA. Also, handling of default props is not working as nice as in React or in ivi (as Daniel aka. FredyC has pointed out above, this is something where the new React API really shines) - but I think it's still okay.

Some of the demos use a special props validatation feature - actually not really important here, just to give you a bigger picture...

Be aware that there are no built-in hook functions in the core - every hook function is therefore a custom hook.

Hello World: https://codesandbox.io/s/fast-grass-juk0q

Clock: https://codesandbox.io/s/wandering-cdn-749i3

Simple counter: https://codesandbox.io/s/infallible-turing-kbn7q

Stopwatch: https://codesandbox.io/s/reverent-raman-xnssv

brucou commented 5 years ago

The goal of this fun library is to find a completely magic-free, old-school technique based way of implementing components where the render functions shall be pure (the latter in difference to React and ivi).

What makes you say the render functions in your examples are pure? I see a using(fn) where fn indeed looks pure, but do you mean that using is also pure? I assume using has the same signature than fn. Is that correct? Anyways, from just looking at the examples, it is impossible to be positive about purity.

Then what benefits would you derive from that pure render function? I am not sure if you can extract it out of the component for instance for testing purposes if using is coupled to the component:

[v, using] = wrapGetters({
        props: useProps(c),
        state: getState
      })

That aside, the whole thing looks pretty similar to the new Vue v3 syntax where you register callbacks at different stages of the lifecycle when the component is initialized.

spion commented 5 years ago

Hope you do not mind if I write an additional comment to this topic after all this time, even by running the risk that you guys have already completely lost interest in this stuff here

Please feel free to continue. The more I use hooks, the less I like them and the more they seem like a mistake to me, so I'm looking into alternatives!

Are you sure its necessary to combine the function based API with the properties API? For example, why not have usePropsValidation(props, validator); or usePropsDefaults(props, defaults) ? That way you can get rid of the object and pass just the function instead

mcjazzyfunky commented 5 years ago

@brucou Many thanks for your feedback. You are right, my wording was not accurate enough. The "render function" in the stateful components above is strictly speaking the return value of the "init function" and therefore surely not pure. Actually, I meant the inner function of that using invocation above, which is pure in these examples ... but may not necessarily be pure in more complex use cases. Nevertheless you can in general always prepare view data inside of the (as you said) actually non-pure render function and then call a real pure renderMyComponent(viewModel, ....). But you can do the same in React and ivi, so my statement above was not correct and I will strike it out. Thanks for that hint.

mcjazzyfunky commented 5 years ago

@spion TYVM for your response

Are you sure its necessary to combine the function based API with the properties API? For example, why not have usePropsValidation(props, validator); or usePropsDefaults(props, defaults) ? That way you can get rid of the object and pass just the function instead

Actually, I am "sure" of nothing :smiley: ... but it's actually not "necessary" ... that object configuration thing is just a matter of tase ... I just thought it's better readable and less verbose than always importing and chaining the same component helper functions. Also, with this API you have to distinguish somehow between stateless components (=> render function) and stateful components (=> init function), and I just thought a component config object may be a good way to do so. Oh and please be aware that this special library here tries to have better support for "not-using JSX", all components can be used out-of-the-box like follows (that's why in all cases you need this component function):

Counter({ initialValue: 100, label: 'My Counter' })

which has exactly the same result as

<Counter initialValue={100} label="My Counter"/>

Regarding usePropsDefaults(props, default): Actually a former implementation of this useProps hook had a second paramters for the default props useProps(c, defaults). The problem is that this c object has a function c.getProps() if you do not want to pass the default props somehow to the component function then you should remove that c.getProps() function from the interface of c and pass getProps or something similar as second arguments to that init function. Otherwise with one hook you get raw props with the other you get defaulted props - which may not be want you want (BTW: It makes a lot of sense that in React hooks do not have access to props - but here things may be a bit different). Oh btw: Your proposal to use useDefaultProps is way easier to type in TypeScript than my API due to some current limitions of TS's type system (there's a reason why component(displayName)(config) is curried - this is due to those subtle TS typing limitations).

Regarding usePropsValidation: I am very strongly of the opinion that the props validation should be done when the virtual element is created (as currently done by React and also by this fun library of mine) and not when the component is rendered. .. which gives some flexibility in some special use cases (components that do not render themselves but will be rendered by their parents or will just be used as data - I was told this was an anti-pattern ... maybe, but I am not really convinced). Moreover the props validation spec is IMHO a very elementary core feature of the component while those hook functions are just some helper functions therefore this props validation feature should be somehow part of thatjs-widgets core module and not in one of the not-so-important add-on packages. Also, I think that validation configuration is more easily removable automatically for the production build by some special Webpack/Rollup plugin or whatever.

mcjazzyfunky commented 4 years ago

I know comparing simple code snippets is not really meaningful. Nevertheless I've implemented that simple StopWatch example in all three patterns that we have discussed here to allow at least a very basic comparision.

To summarize: While I still prefer the React approach, I am really strongly of the opinion that the two factory pattern variants (@zouloux's POC and the approach that ivi is using) are also nice solutions (IMHO, both are waaaaaay better than using React.Component - and not very long ago, I'm sure, 98% of all React users were completely fine with React.Component).

Frankly, I do not see anything regarding that factory pattern alternative that has deserved all that beef, bullying and "thumb downs" that that pattern has met in the long React RFC no. 68 discussion. Moreover, I cannot see any longer syntax that is that verbose that it makes you dizzy.

  1. React:

    Please be aware that there's actually a little bug regarding props.name in the useEffect invocation of StopWatch. Fixing it would not be a problem but that would cause some additional lines of code which would impair comparability. And actually I think it's interesting that you do not have such an issue with the "Simple factory pattern" described below.

    https://codesandbox.io/s/thirsty-hooks-nqgbg

  2. "Simple factory pattern"

    Basically the pattern that Alexis (@zouloux) has presented in his prehook POC, enriched with that ivi-like c thingy and replaced the access mechanisms for varying data from props.value.someProp and someState() to a proxy based solution. It is surely arguable whether proxies are really needed here, nevertheless it's syntactically the most concise solution that I can think of, and we have not mentioned this solution yet, so I gave it a try.

    Please be aware that this approach has more the mental model of React classes than of React hooks.

    https://codesandbox.io/s/ecstatic-hawking-zm9cr

  3. "Advanced factory pattern":

    This uses one of the patterns that Boris is using in ivi:

    [...] outer function should create dataflow pipelines and inner function should wire dataflow pipelines and pass input data.

    Please don't be confused that I've changed the naming convention a bit (handleABC / useXYZ).

    Same bug as in the React version (see above) and also the same reason why I intentionally do not want to fix that bug here.

    https://codesandbox.io/s/silly-bird-h6fze

mcjazzyfunky commented 4 years ago

Okay, and last but not least, here are some of the demos that Tim has prepared for that React hooks RFC discussion (https://codepen.io/timkendrick/full/wObdyb) implemented with the above mentioned "Simple factory pattern" (as said, basically the pattern of this prehook POC plus some extras).

https://codesandbox.io/s/crimson-brook-dx92d Not really something new here, but I think it's nice to have as many examples as possible.

When I migrated that code to the other pattern, I thought that Tim's ideas just needed two further steps and they would be similar to that above mentioned "Advanced factory pattern" that Boris is using in ivi and all those issues that Dan had mentioned in his response would disappear ... to repeat Boris' approach again:

[...] outer function should create dataflow pipelines and inner function should wire dataflow pipelines and pass input data.

I had to laugh when two hour ago I've seen that Rasmus (@mindplay-dk) has already written something similar half a year ago:

[...] It's one step closer to the ivi-approach [...]


Regarding that "Simple factory pattern":

There's this comment by Dan about solutions where hooks have access to the component's props:

But it seems like this breaks encapsulation quite a bit because any custom Hook now suddenly has access to its owner component's props. That makes them non-compositional.

Currently that little ivi-like c thingy that I am using in my examples allows access to the current props. Not really sure if Dan's comment is also equally true for my interpretation of that "Simple factory pattern". If yes, the solution would be to remove it from c and pass getProps as second argument to that init function (but I would not like that too much).

Secondly, the React team (especially Dan) are always pointing out that the mental model of React hooks fits better the demands of React's concurrent mode than the mental model of component classes. As that "Simple factory pattern" has IMHO the same mental model as component classes, I'm not really sure whether this is a showstopper for that "Simple factory pattern" or whether the whole thing is just a matter of taste. If props is a proxy for changing data then of course this is a bug:

useEffect(() => {
  ChatAPI.subscribeToFriendStatus(props.id, handleStatusChange)
  ...

  return () => {
    ChatAPI.unsubscribeFromFriendStatus(props.id, handleStatusChange)
  }
}, ...)

and would have to be replaced by

useEffect(() => {
  const id = props.id

  ChatAPI.subscribeToFriendStatus(id, handleStatusChange)
  ...

  return () => {
    ChatAPI.unsubscribeFromFriendStatus(id, handleStatusChange)
  }
}, ...)

But is this really a showstopper?

BTW: Without proving it: With that "Simple factory pattern" hooks need a bit more effort to be implemented than with the two other pattern. But manageable.


Regarding the ivi-like "Advanced factory pattern":

As this has the same mental model as React hooks (at least if you apply that pattern accordingly), there should not be problems regarding concurrrent mode or maybe even theads in some future JavaScript versions. What I really like in the ivi approach is that the counterpart to React hooks' "dependency" arrays are just the arguments passed to the corresponding functions. Feels more natural and doesn't need a linter that evaluates the dependencies. What I do not like that much with that ivi-like "Advanced factory pattern" is that you have to provide more type information explicitly in the initialization block with TypeScript (try it out and you will see what I mean) and also the handling of more complex callback functions is a bit tricky (see: https://github.com/solid-js/prehook-proof-of-concept/issues/1#issuecomment-478175905) ... I think I would just add a ivi-like counterpart to useCallback to handle the latter (but that's IMHO not that nice as in the "Simple factory pattern", where callback functions are basically trivial).

mcjazzyfunky commented 4 years ago

For the sake of completeness, here are the six examples from the last comment implemented with the "Advanced factory pattern" : https://codesandbox.io/s/reverent-sara-oe4ch

Please have a look at Demo-4 "Using callbacks": Here the implementations of the two callback functions onInputand onButtonClicked are completely different (feels a little odd, doesn't it? .... that's what I've meant in my latest comment). Also please be aware that at initialText =>... and text => ... you would have to provide additional type information in TypeScript which you do not have to do for the same use cases with React and with the "Simple factory pattern" (this was also mentioned in my latest comment).

BTW: That "Loading async data" demo seems to be a bit buggy - not really that important, I think, maybe I've also made the same mistake in the previous demo.

zouloux commented 4 years ago

Hi guys, long time no see :) It looks like Preact is looking for an alternative functional system involving factory pattern : https://twitter.com/cztomsik/status/1207349848317595649

mcjazzyfunky commented 4 years ago

I really love that Porfírio "Preact Composition API" guy :heart: :heart: :heart: ... he really breathes new life into the whole discussion about alternative hook concepts. Actually, his proposal is what I've called above "simple factory pattern" plus some of Vue-like reactivity stuff, isn't it? Porfírio's implementation seems to be optimized for small output bundles (what I think is quite important in the Preact community), so the change detection mechanism he has implemented is by design limited to the most necessary (no deep change detection etc.). I personally would consider to use something like @nx-js/observer-util which adds around 3kb but has a sophisticated deep change detection included and could also be used outside of the compoenent, for example for the app state and general change detection if wanted (and btw: the implementation of a proper observing and memoizing watch/compute function would be mucht easier) .

But what really, really puzzles me completely is: Why is an API like the following currently considered as "the cool new kid in town" ...


const Counter = statefulComponent('Counter', () => {
  const
    state = object({ count: 0 }),
    onIncrement = () => state.count++

  observe(() => {
    console.log(`Value of counter is now ${state.count}`)
  })

  return () =>
    <div>
      <label>Counter: </label>
      <button onClick={onIncrement}>{state.count}</button>
    </div>
})

// see: https://codesandbox.io/s/heuristic-visvesvaraya-4uhti

.... while the following API is not regarded attractive enough to even discuss about???

const Counter = statefulComponent('Counter', c => {
  const
    [state, setState] = useState(c, { count: 0 }),
    onIncrement = () => setState('count', it => it + 1)

  useEffect(c, () => {
    console.log(`Value of counter is now ${state.count}`)
  }, () => [state.count])

  return () =>
    <div>
      <label>Counter: </label>
      <button onClick={onIncrement}>{state.count}</button>
    </div>
})

// see: https://codesandbox.io/s/upbeat-lalande-cyu3x

Is there syntactically really that much difference?

zouloux commented 4 years ago

Yep, puzzling at first but maybe this is just a "reactive programming" rethinking which can be more accepted since our solution is more in-between pure functional and reactive. The first concern which made me create this repo was "oh, React API is cool, but it can be better on some ways", (and the same for a lot of people from the React discussion, not just me), we added a render function to optimize calls and we encountered some issues. Also I think we sticked too much to the React syntax of use* and stuff, some ideas here are really going beyond that which is really cool ! When I see Svelte for example https://svelte.dev/blog/write-less-code, we see there are a ton of new ideas / patterns for this kind of reactive paradigm. About Svelte, the new Syntax sugar API is a no go to me (for now), because it is too far away from Javascript but what it offers is crazy. What do you think ?

mindplay-dk commented 4 years ago

@mcjazzyfunky I'm so completely with you on this one! Why is the obvious alternative regarded as "unsexy", while solving the problem with side-effects on global state is considered the new New? These are things most people would have shunned a year ago, but React is doing it, "so it must be cool". 🙄

(see also this little toy experiment of mine...)

mcjazzyfunky commented 4 years ago

@mindplay-dk Thanks for the demo. I have some question regarding this:

1.) Let's say you want the counter to have a default value of 100 and a default title of 'Counter'. Would you do the following or what else would you do?

Counter.defaultProps = {
  title: 'Counter',
  value: 100
}

2.) In line 224: Let's say you also want to print out the title of the counter. Would you do the following or what else would you do?

console.log(`You clicked "${counter.props.title}" ${state.count} times`)

3.) Let's say you want to write a hook function useWindowSize what TypeScript type would that function have? Same question for a hook functions that returns a bundle of 10 different things.

Thanks a lot in advance.