oncreate vs onupdate

[mindplay-dk]

I noticed that onupdate doesn't fire during the first update, is this intentional?

I think, in most systems that offer both a create and update notification, the initial creation triggers both a create and an update notification, because there isn't typically much use for an update notification that doesn't fire the first time.

Typically the notifications you need are for creation, for destruction, or for all updates - rarely does anyone need a notification for "all updates except the first one".

Thoughts?

[jorgebucaran]

I think, in most systems that offer both a create and update notification, the initial creation triggers both a create and an update notification because there isn't typically much use for an update notification that doesn't fire the first time.

I've noticed sometimes you need to use both oncreate/onupdate for the same thing and sometimes don't. When you have to, it's inconvenient writing a function with some logic and calling it in two different places.

in most systems that offer both a create and update notification

Do you have a concrete example?

Also cc'ing this to @zaceno.

[mindplay-dk]

I've noticed sometimes you need to use both oncreate/onupdate for the same thing and sometimes don't.

That's what I mean. I think this change would take care of that?

I guess it's a matter of what you think "on update" means - does it mean "updating the element", or "updating the attributes of the element"?

I think the latter makes more sense - because we don't really "update the element"; there's nothing you can update, other than it's properties and attributes, so we're really "updating the attributes of the element", which happens both after creating and updating the elements.

Can you think of a use-case where you need to do something when the attributes get udpated except the first time? I can't. Almost every use-case I can think is either creation only, or every update.

[jorgebucaran]

Can you think of a use-case where you need to do something when the attributes get udpated except the first time?

I can't either. 🤔

[zaceno]

@jorgebucaran @mindplay-dk

I haven't had situations where I need to do something on update except the first time.

What I have found, are usually I have to do the update-thing + something extra in oncreate.

I suppose it could work as well to have onupdate run also the first time, but there would have to be a predictable order, like: oncreate always runs before onupdate.

Except that will become tricky if, when the element is first created I want to do:

1. some stuff
2. the same stuff I want done every onupdate
3. some more stuff

In that case I'll have to move 2-3 into the onupdate, and use an If statement to detect if this is the first run of onupdate and only do 3 in that case.

This might not be very common though.

I can see how running onupdate also after oncreate would be quite practical in many cases. But personally I like the clear distinction we have currently: this happens the first time, and this happens all other times.

[jorgebucaran]

What do you think @SkaterDad?

[SkaterDad]

I haven't used onupdate for anything yet, so I'm not going to be opinionated here.

Having separate oncreate and onupdate hooks seems pretty logical. I wouldn't be opposed to running onupdate directly after oncreate, but @zaceno brings up some good points on where that could trip people up. If your oncreate and onupdate functions need to do the same thing, it's only a couple lines of code, so I'm not seeing a problem.

[mindplay-dk]

I can see how running onupdate also after oncreate would be quite practical in many cases. But personally I like the clear distinction we have currently: this happens the first time, and this happens all other times.

The most common use-case I can think of, is when integrating with a third-party JS widget - this will need to be updated every time, e.g. both when creating and patching.

So perhaps there's a use-case for another event that only happens when an existing element is patched?

So like:

• onupdate would fire every time the properties/attribtues of an element get updated
• onpatch would fire only when the update patches an existing element (current behavior of onupdate)

Make sense?

tbh, I can't see myself ever actually using onpatch - the execution order problem described by @zaceno, like he said, probably isn't very common.

it's only a couple lines of code, so I'm not seeing a problem.

@SkaterDad yeah, but it's a couple of lines of code what seems to be the majority use-case - it's fine to also support minority use-cases of course, but in my opinion the order and scope of events should be tailored so they match the majority use-case.

[jorgebucaran]

The only way to figure this out is by coming up with an actual use case, whatever it may be.

[mindplay-dk]

The only way to figure this out is by coming up with an actual use case, whatever it may be

Well, one thing I'm trying to figure out is how to deal with components that need internal state - I'm almost positive we'll need the "on every update" hook for that.

So for example, I built this date-picker as an exercise on friday:

https://jsfiddle.net/mindplay/6czx4dmg/

It went smoothly, and I'm very happy - but the question is, where does internal state go? For example, _offset in this model is the current +/- offset in months from the currently selected date, which is strictly interesting to this widget internally - no one will ever need to know about this state.

How can I turn this into a reusable component? Without forcing client views to deal with the irrelevant offset value, e.g. by maintaining both the value (selected date) and the offset as state in their own model?

This is the big question for me right now.

I love building individual components in this manner, but it's no real use unless I can also turn a widget like this into a reusable component.

I've been trying to figure out how to use oncreate, onupdate and ondestroy in practice to implement private component state, but haven't come up with anything so far.

I guess what I'm looking for is a "mimimum viable component" pattern with Picodom - not so much a feature or a framework, but just the pattern itself.

No doubt this is going to depend on an update hook that fires with every update, and as mentioned, so will integration with third-party JS widgets.

Those are currently the two hottest use-cases for me, as this is what will make it possible to scale to rich, complex UI and reusable widgets.

Any suggestions would be more than welcome :-)

[jorgebucaran]

@mindplay-dk That's awesome, but I mean what would be the use case for running code on onupdate, but not on oncreate or at least not the same code.

I was just thinking how in Hyperapp, where there is ssr re-hydration, oncreate won't be called after re-hydration (since the vtree already exist) and only onupdate is. Your proposal would not affect that behavior, just saying.

@zaceno What is a valid use case for running code only on onupdate? Just wondering.

[zaceno]

@jorgebucaran I don't know of any use case for running code only on onupdate - what I was saying before is rather that there are good use cases of this:

• oncreate does X + Y (or Y + X ... the order may vary)
• onupdate does X

@mindplay-dk Perhaps a bit OT, but you might be interested in seeing how I solved the problem of stateful components in hyperapp, here: https://github.com/zaceno/hyperapp-nestable. Yes it's hyperapp, but the same idea likely holds for picodom + whatever state manager

Actually not so OT come to think of it, because see the src: https://github.com/zaceno/hyperapp-nestable/blob/master/src/index.js ... It's actually a perfect example of oncreate doing onupdate+more.

It's also a good example of how things would be more difficult (with this solution anyway) if onupdate ran when the element is created as @mindplay-dk suggested. Because it would mean:
1 ) I must be sure that oncreate runs before onupdate
2) In onupdate I need to check somehow if this is the first run of onupdate so I can do if (_actions.init) _actions.init() (oh and I'd need to save the init function on the element too)

not terrible, and it could work. But this why I think I prefer things the way they are.

[zaceno]

@mindplay-dk Regarding your suggestion of adding an onpatch (which behaves like todays onupdate, and make onupdate behave like you suggested):

We experimented with your suggested type of onupdate (only run when props change) but eventually went back to just every patch.

I think the reason was that it can be kind of hard to know (in picodom) if the props really were changed (we'd have to do deep comparisons, and functions are just impossible to tell) -- so better to just assume everything has changed and leave it up to userland to do the comparisons in an informed way. @jorgebucaran can probably explain it better.

Anyway, I was happy to go back to the current behavior, because in hyperapp-transitions (which also work with picodom. I've added examples in my README.md now) I need to listen to an event which would be called for every transitionable-element, every time the layout may have changed. So basically I want a callback whenever anything anywhere has changed -- i e, the current onupdate behavior.

EDIT:
If we want to continue talking about the idea of onupdate running only when properties has changed, I'm not opposed to that (as long as we keep something like @mindplay-dk 's suggested onpatch. But that's a separate topic, so: new issue for that, I think.

[jorgebucaran]

Currently

• createElement → oncreate
• patch/updateElement → onupdate

@mindplay-dk's suggestion

• createElement → oncreate → onupdate
• patch/updateElement → onupdate

[mindplay-dk]

So, taking in @zaceno's comment, my current suggestion is:

• createElement → oncreate → onupdate
• patch/updateElement → onpatch → onupdate

In summary, oncreate and onpatch are mutually exclusive - "create" and "patch" are about elements being created or patched, while "update" is about properties/attributes being updated, which happens both when elements are "created" or "patched".

But that's a separate topic, so: new issue for that, I think.

I think we should deal with this change as one PR - updating the documentation will be important, and changing the behavior and description of onupdate doesn't make sense (per @zaceno's comments) without also introducing and describing onpatch.

Let's get the changes, documentation updates, and tests, into a single PR, so that the project doesn't end up in an intermediary state that doesn't make sense?

[jorgebucaran]

Or we could leave things the way they are.

[zaceno]

@mindplay-dk I agree that any changes that belong together should be PR:ed together, but for just discussing how things should be, issues should be used (like here). And separate questions should be in separate issues or we'll have trouble reaching agreement on anything.

Now I realize I might have misunderstood you. First I thought you were suggesting that onupdate would only run when any of the components props change. Like I said, we've tried that and it's a fraught topic. So if that was what you were suggesting it's a whole other can of worms worthy of a separate issue.

But (at least for this issue) you're just talking about the timing -- right? What happens the first time an element is created, and what happens every time the patch function is run for the element. Wether or not props have changed is not relevant. Is that correct?

[zaceno]

@mindplay-dk @jorgebucaran

I'm on board with @mindplay-dk 's latest suggestion -- that works for me. But I'm also OK with leaving things as they are.

[mindplay-dk]

First I thought you were suggesting that onupdate would only run when any of the components props change

To be clear: no, I'm suggesting onupdate run consistently for every update of attributes, whether that update applies to a newly created or patched element, and whether or not any attributes have actually changed - because that seems to be the most common use-case.

For one, we'll need this functionality if we're going to integrate with third-party JS widgets - for example, a date-picker would need to be updated, always, whether the element was just created or is being patched.

Now, another thing occurs to me though - functional components do get invoked with every update, so the function itself also kind of works as a "hook" for every update. The problem with that one is, the component has no access to it's prior state, and no means of telling whether the current call will trigger oncreate.

My main interest in this subject to begin with, is figuring out how to build reusable components. My date-picker example makes for a real use-case: how would you turn this into a component? I've been struggling to figure this out, and because the create/update/destroy hooks do not have access to any common context, I'm stuck with this one.

Is it possible with the current life-cycle hooks? Would love to see an example.

[jorgebucaran]

I don't want to introduce a new lifecycle event.

So, if:

• createElement → oncreate → onupdate
• patch/updateElement → onupdate

does not disrupt anything, I am probably okay with it. But I would like to make sure this does not break anything.

[mindplay-dk]

does not disrupt anything, I am probably okay with it. But I would like to make sure this does not break anything.

It's a breaking change, of course - if you were manually calling the same function from oncreate and onupdate, you'll need to fix that.

@zaceno can you look at your use-case and try to determine if it could be implemented with this change?

If so, I'd say, let's go ahead and change onupdate - if there is no current use-case for a new onpatch, we can cut it; if a use-case emerges, we can consider introducing it then.

[jorgebucaran]

It's a breaking change, of course - if you were manually calling the same function from oncreate and onupdate, you'll need to fix that.

That's okay, I can accept that. But will it make impossible to do something else?

[mindplay-dk]

But will it make impossible to do something else?

I just had an idea, let me just to try something out before we go ahead with this change - I want to check whether this might block a component implementation pattern I just thought of...

[jorgebucaran]

@SkaterDad @zaceno If we change this behavior, it will make it to Hyperapp as well, so I'd like to make sure (1) we want this (2) it does not prevent you from doing something else and (3) we won't end up introducing another lifecycle event later.

[zaceno]

If we dont add a lifecycle method to behave like onupdate currently does, I am opposed to this change.

It Is a breaking change, for one. It doesn't make anything impossible to do afaik - but it does make at least one of my uses more cumbersome.

If @mindplay-dk is just trying to work out how to make stateful, reusable components, let's just help him with that.

The lifecycle methods do have one thing in common, upon which state (or a whole state management system) can be attached - namely the element.

[jorgebucaran]

@zaceno It Is a breaking change, for one. It doesn't make anything impossible to do afaik - but it does make at least one of my uses more cumbersome.

Could you explain your use case to me? I still don't get it.

[mindplay-dk]

If @mindplay-dk is just trying to work out how to make stateful, reusable components, let's just help him with that.

@zaceno thanks 😄 yeah, that is my primary pursuit at this point.

I just found an article here talking about how to attach state to a functional component in React:

https://medium.com/@dai_shi/attaching-state-to-stateless-function-components-in-react-db317a9e83ad

Unfortunately, the library relies on React Component to implement it:

https://github.com/dai-shi/react-compose-state/blob/master/src/index.js

I can't think of a way to preserve state between calls that doesn't involve either:

1. Injecting state into the DOM elements (since the old VNode is inaccessible prior to first call)
2. Using a class/instance for the component model - the application model would need to create component models and then pass them to the view-functions, but, this involves creating and managing the model instance anyway, which doesn't seem any better/worse than class-based components.
3. Introducing class-based components to Picodom, which I'm sure you'd be rightfully opposed to ;-)

I wrote a bunch of example components on friday, using a pub/sub pattern where the model broadcasts a notification on model invalidation - then using a render-function to update the view.

I like this approach, but the next step is to make it reusable, preferably using JSX syntax, so, something like <DatePicker value={ model.start_date }/> in your application, with the expectation that the date-pickers internal state will initialize and be preserved - internal state such as _offset in my example, which designates the current offset from the selected date, in months, which is of no interest to the application itself; it wouldn't care about anything other than the selected date.

I don't understand how that can be done using functions only?

[jorgebucaran]

@mindplay-dk Why are we discussing this on this issue though?

[SkaterDad]

@jorgebucaran I'm in agreement with @zaceno, that altering the oncreate behavior to also call onupdate is a breaking change. I like the way it acts now, personally, since the lifecycle names tell you exactly when they'll be called.

[zaceno]

@jorgebucaran Could you explain your use case to me? I still don't get it.

See the link I posted above : https://github.com/zaceno/hyperapp-nestable/blob/master/src/index.js
Never mind what it does. Just try to rewrite it to have the same behavior with the suggestion of running onupdate immediately after oncreate and you'll see the problem. It's possible, but gets a lot more complicated, because you'd need to have some code in onupdate which should only run if this is the first call to onupdate -- and you need a way to know that.

EDIT: Actually sorry. You do need to mind what it does :P. the crux is this:

            ...
            oncreate: function (el) {
                ...
                el._$(props)
                if (_actions.init) _actions.init()
                ... 
           },
            onupdate: function (el) {
                el._$(props)
            },

So el._$(props) sets the properties passed to a component on its internal state. That has to be called before if(_actions.init) _actions.init() -- or else there won't be any initial state for the internal init action to work with.

So, if I leave it as it is, then with the proposed change, the order things would happen is this:

//in oncreate:
el._$(props)
if (_actions.init) _actions.init() 
//in onupdate:
el._$(props)

If the init action needs to do anything to the state-properties that were provided from the outside, those changes will be overwritten immediately by the second el._$(props)

So, I could change things to be:

            ...
            oncreate: function (el) {
                ...
           },
            onupdate: function (el) {
                const virgin = somehowFigureOutIfThisIsTheFirstUpdate(el, props)
                el._$(props)
                if (virgin && _actions.init) _actions.init()
            },

... the trouble being, besides the conditional code, how to implement somehowFigureOutIfThisIsTheFirstUpdate

[zaceno]

@mindplay-dk

I took your datepicker js-fiddle and turned it into two instances of a reusable datepicker component such as you wanted. Have a look: https://jsfiddle.net/1vhetmo5/4/

NOTE: check the console also, for stateful components to be useful, you usually need something out of them, so I gave the component an onselect callback, which the main app binds to console log, so you can see it works

NOTE 2: My bad, here's the up-to-date link with the onselect behavior. https://jsfiddle.net/1vhetmo5/6/

[mindplay-dk]

@zaceno thanks, but this is what I suggested not to do in (2)

Using a class/instance for the component model - the application model would need to create component models and then pass them to the view-functions, but, this involves creating and managing the model instance anyway, which doesn't seem any better/worse than class-based components

There are two reasons I don't like this approach:

1. As said, it involves creating and managing an object instance anyhow - and manually at that, so probably overall worse than class-based components in one sense.
2. We're replicating the problem we had with DOM elements in the first place - and the solution is more or less the same as you would have written if you were writing plain DOM code, the only difference being this is not a DOM element but some other component.

You renderer function is interesting - it's a lot like the mount() function you find in various frameworks? You also managed to achieve individual rendering of component instances, which is very cool.

You're injecting the model into the DOM element, which may be problematic. I don't know if circular references still lead to memory leaks in modern browsers, but people invented things like .data() in jQuery to avoid problems that could arise from this approach, I think?

I don't want to hand-code this kind of thing every time though, I don't think anybody does - that's probably why classes are popular? It would be too verbose and error-prone.

So maybe I am looking for something more framework-ish after all. Has anyone built anything on top of Picodom yet? I started something a while back (mainly to get support for class={{ foo:true }}) but I didn't publish anything. Perhaps it's time to start talking about a "plus" layer as a separate package, with support for stateful components, a mount() function, etc.?

[jorgebucaran]

It seems we are no longer talking about the original issue anymore. And it seems the conclusion we reached is to add more lifecycle events or keep things the way they are, so I am going to keep things the way they are for the time being.

@mindplay-dk Has anyone built anything on top of Picodom yet?

I found a few dependents of Picodom. I don't know how actively maintained those projects are, but maybe someone already figured out stateful components they way you envision them.

[zaceno]

@mindplay-dk thanks, but this is what I suggested not to do in (2)

Hehe ... you wrote that model as a class, not me 😄

I wouldn't have used classes either but I was just trying to do "minimally invasive surgery" to demonstrate the technique. The key part is how I implemented the DatePickerComponent. Basically, it's like a mini app attached to the main app.

Yes that involves binding the state-manager (instance of DatePickerModel) to the element. I can't think of any better way and I'm pretty sure it's safe (unless you do something wicked with eventlisteners) -- but if you find a concrete problem with it be sure to let me know.

BTW from the list of picodom dependents @jorgebucaran linked to, I know that frapp is maintained and capable of stateful components in some form (basically the same idea as my implementation above: composing many small apps into a bigger app). But not much documentation from the looks of it. Still better than my entry zx-app which has zero docs 😆 . ( But that's intentional... it works well, and you can look into the code if you like, but it's not for public consumption just yet)

[mindplay-dk]

Hehe ... you wrote that model as a class, not me 😄

To be clear, I don't have a problem with classes 😃

But if the solution involves a class (or any object model) the framework needs to somehow manage the life-cycle of the object internally - it should be an implementation detail, not something that forces the consumer of the component to deal with the model and view independently. What makes this error-prone (and confusing) in my opinion, is the need to manually wire together two ends of the same thing - the component must be readily usable.

Here's another possible approach I just thought of: the call to the functional component itself has access to props, which may be a better option than injecting state into the DOM element - if we inject something into props, it should carry over via oldProps which is made available to onupdate, which can then forward inject it into the new VNode.

Creating a general facility like this one might then be possible??

[jorgebucaran]

@mindplay-dk Have you considered joining Hyperapp's slack? There's an entire room dedicated to picodom.

[mindplay-dk]

@jorgebucaran there's a slack? There's no link to it anywhere 😆

[mindplay-dk]

By no means "there" yet, and I have to quit for today, but I did make some interesting progress.

https://jsfiddle.net/mindplay/o2rdL259/4/

I've extended the h function so I can test if type is a constructor that extends Component - right now it's recreating the component on every iteration, but so far so good. Next step is to try to inject the component instance into a reserved field in props, and make it carry over via oldProps, which should be possible by proxying oncreate and onupdate.

The sample class here illustrates what I'm trying to do - I think that class-based components do make sense here - you'd be able to encapsulate the model and the render function into a self-sufficient, reusable component. It's not exactly the React approach, as I'm putting the component itself in charge of managing it's own state and calling Component.invalidate() to re-render. We'll see if this pans out... :-)

[jorgebucaran]

@mindplay-dk Yes (the picodom channel is new, hence no link), and you are welcome to join! 🍪

[mindplay-dk]

Hmm, okay, I don't think class-based components are possible with what we have now.

The problem is, patch only lets you patch the entire contents of an entire element.

Components can occur anywhere in a list of child elements, and need to render invidually.

The oncreate and onupdate hooks aren't invoked until rendering actually happens, so the Component class, during the call to h, can't know whether it should construct a new instance or create a new one.

So the only possible work-around right now, is to wrap every component in a dummy div, which is obviously not the way to go.

Short of forking and modifying patch to actually support calls to a render method, I can't think of a working approach - since patch is internally recursive, proxying patch doesn't make any sense and won't work. (you'd have Component support only during the immediate call.)

I mean, there's one approach, which is to replicate the entire diff/patch algo and recursively handle Components before calling the actual patch-function, but that hardly makes any sense?

[rbiggs]

Hey mindplay, you need to move the state into the component. Talking about state getters and setters. On top of that, state needs to be in another semi-private class property that the accessors use. That way this gets inherited by all extensions of the Component class. You would also want to store a reference to the parent, oldNode elements directly on the Component class. That way each instance has its own reference to its parts to use when patching. And finally, patching is done with setting state. I put together this Codepen to illustrate: https://codepen.io/rbiggs/pen/opqyEM?editors=0010. With this approach, when you create a stateful component and instantiate it, it automatically gets rendered to the DOM. Also updating its state will patch the DOM again. Missing, add a setState method that can accept either data or a callback that gets the component state.

This is very basic but shows that you can create class components that work with Picodom without a problem. This gives you basically a React-lite. Having component instances handle patching makes it easy to create multiple instances of a component with different state.

[jorgebucaran]

Thanks, @rbiggs. Long time no see! 👋

I am going to ping @mindplay-dk just to make sure he gets this notification.

[mindplay-dk]

@rbiggs yes, this is what I've been doing, more or less.

https://jsfiddle.net/mindplay/o2rdL259/

I use a simple pub/sub to decouple the model from the view, but this is otherwise similar.

This works great for individual pieces of UI, but it doesn't work well for composite views - and it doesn't enable JSX syntax for components, you have to manually manage the life-cycle of each component.

We're discussing it further here.

[rbiggs]

With Preact, Inferno and React, when a component first renders only componentWillMount and componentDidMount fire. After that, when the component is updated componentWillUpdate, componentDidUpdate and componentWillReceiveProps fire.

It doesn't make sense to me that you would expect an update event to fire when a component is first created. It's not being updated, it's being created. Two very different scenarios. Firing update when a component gets created feels like muddying up what update is. Update should be for when there's a change to the current state of a component. If the component doesn't exist yet and is being created, then there is no old state to update.

This brings me to another thing I've noticed with Picodom: onupdate gets fired event when the component doesn't actually update. This happens when you rerender a component with the same data. In the updateElement function you check to see if the old and new properties match before updating an element:

if (props[i] !== oldValue) {
    setElementProp(element, i, props[i], oldValue)
}

But then, event when you short-circuit this be exiting when the props are the same, you still push onupdate to the callback stack:

if (props.onupdate && shouldUpdate) {
  callbacks.push(function() {
    props.onupdate(element, oldProps)
  })
}

Seems like this is weird. Why do you execute the onupdate callback with every call of updateElement but check to see if the props are different before actually updating an element?

I would not expect an onupdate callback to be executed when the element wasn't actually updated. You could add a check in there, something like shouldUpdate:

function updateElement(element, oldProps, props) {
  var shouldUpdate = false
  for (var i in copy(oldProps, props)) {
    var oldValue = "value" === i || "checked" === i ? element[i] : oldProps[i]

    if (props[i] !== oldValue) {
      shouldUpdate = true
      setElementProp(element, i, props[i], oldValue)
    }
  }

  if (props.onupdate && shouldUpdate) {
    callbacks.push(function() {
      props.onupdate(element, oldProps)
    })
  }
}

With this check, onupdate would only get pushed to the callback stack when the element actually gets updated. I dunno. Maybe I have too much coffee in my system tonight. ☕️👨🏻‍💻

[jorgebucaran]

@rbiggs onupdate will always fire, but we expose the oldProps so you can figure out if something actually was updated or not.

[zaceno]

@rbiggs we actually had onupdate working like you suggested for a while, but it didn't work out well.

For one, there might be reasons (arguably mischievous, but anyway) to want a hook/callback to do something in a component any time anything anywhere changes.

For two, since you'll mostly be writing your event handlers like onclick: ev => actions.buttonClicked(), -- that means function instances will be new every time. And there's no really good way to check equality of functions when they have different instances. So in practice, most components (though not all) ended up getting onupdate:ed anyway.

Basically: the user is better equipped to state the conditions of when the onupdate should be run, than hyperapp can be. Hence @jorgebucaran 's suggestion above.

Another (even better) trick you could use is the fact that patching is entirely skipped over if a vnode is the same instance as the old one. That means you could memoize a component based on the props passed to it. If the props haven't changed you return the vnode in the memo (same instance as before) and no patching and no lifecycle callbacks are called for the entire subtree. (It's kind of like our version of React's shouldComponentUpdate)