Hyperapp: oncreate appears to be broken

[FlorianWendelborn]

OnCreate seems to be broken currently. When porting a rather large application from [email protected] to [email protected] I experienced a crash.

Broken Code [run]

const { h, app } = hyperapp
/** @jsx h */

app({
  state: null,
  actions: {
    bug: element => element
  },
  view: (element, {bug}) => <h1 oncreate={bug}>{element ? 'yes' : 'no'}</h1>
})

Workaround [run]

const { h, app } = hyperapp
/** @jsx h */

app({
  state: null,
  actions: {
    bug: element => element
  },
  view: (element, {bug}) => <h1 oncreate={data => setTimeout(() => bug(data), 0)}>{element ? 'yes' : 'no'}</h1>
})

[jorgebucaran]

@dodekeract Yes, we are not deferring oncreate/onupdate/onremove anymore, but you can use setTimeout there as suggested.

Out of curiosity, why are you updating the state inside oncreate? What's the real use case?

[FlorianWendelborn]

@jbucaran the real use-case is to simulate React's ref behavior. I need to attach an <iframe/> to an arbitrary website and later use a reference to said <iframe/>. To store this reference I use the onCreate event and then I drop it into the state.

Note that I can't really use a hard-coded id since the website may use that id.

I don't think the current behavior looks well-defined/bug-free. We can't expect users to know that using an hyperapp reducer in an hyperapp event will break hyperapp. That's IMO one of the more reasonable use-cases.

[jorgebucaran]

@dodekeract I see. I suggest you don't store the element's reference in the state.

[FlorianWendelborn]

I suggest you don't store the element's reference in the state. — @jbucaran

I don't think that's a good idea. Where else should I store it without creating a second source of truth? The concept of a single state is so good because it's simple and easy to reason about. These benefits fade if you don't force yourself to only use a single central store.

[jorgebucaran]

How would you do it in React/Redux?

[FlorianWendelborn]

@jbucaran This is the feature I'm referring to.

But even if we implement something similar — the main issue still stands. Hyperapp doesn't allow a user to use hyperapp actions in hyperapp events without weird workarounds. That's very unintuitive.

[jorgebucaran]

@dodekeract I still don't understand why you would need to store an element on the state. I believe you should be able to accomplish whatever you are trying to do without storing the element in the app's state.

[FlorianWendelborn]

@jbucaran I don't "need" the element in the state. What I need is a way to get a reliable reference to an iframe whenever I need to send it a postMessage message.

In React, I'd just do this <iframe ref="mycustomfancyframe"/> and could access it via this.refs.mycustomfancyframe IIRC. I'm simulating this behavior by saving the reference to the state.

But this is not the issue we're talking about. The main issue is that hyperapp element events crash hyperapp when you change the state in them. That's quite unintuitive.

[jorgebucaran]

@dodekeract When oncreate is fired, it gets passed a reference to the actual DOM element. At this point in time, the element has been created, but it's not on the DOM, because we haven't had time to call patch yet. Calling an action inside oncreate will update the state (what you want) and try to render the app before its initial default render (breaking things).

What if you just change the state inside the action, but return nothing?

[FlorianWendelborn]

@jbucaran That's an antipattern. Conceptually hyperapp's state is immutable. Being able to do that is an implementation side-effect.

[jorgebucaran]

@dodekeract Now you understand why deferring oncreate works. We could do the same for you under the hood. Or what else do you suggest to fix the issue?

[leeoniya]

in general, keeping references to dom elements can cause mem leaks since they will not get GC'd even when removed or recycled by the lib until you manually remember to unreference them.

[jorgebucaran]

@leeoniya keeping references to dom elements can cause mem leaks

Just to confirm, is this in response to storing references in the state?

[leeoniya]

yeah. if you're storing refs in some persistent state which can outlive the removal of the dom element. you gotta remember to de-ref the ref in your state during onRemove or whatever to let it get cleaned up.

[zaceno]

fwiw, the current behavior of hyperapp to call oncreate once the element exists, but before it is in the dom, is necessary to implement general css transitions (and by general, I mean, that we don't use the app's state to keep track of where the element should be -- only using the lifecycle methods). If the element is already in the dom before I get access to it, I can't add a starting state for an entry-transition.

So, for that case the current behavior is good, and I wouldn't want it changed. To solve @dodekeract's issue, we could consider adding another lifecycle method ("oninsert", "onmount"...), or accept the setTimeout workaround as "the way to do it".

EDIT: Fix mention.

[FlorianWendelborn]

@jbucaran @leeoniya @zaceno This issue isn't about the way I store the reference to an element, that's just an example. This issue is about hyperapp's events breaking when using hyperapp's own actions. There is no reason why this should break anything if I update the state. Instead, one solution would be to defer the next render until the current render is done.

[jorgebucaran]

@dodekeract Sorry to be so rejective, but can't you just create a module with some local variables and use those to track the elements? What I am missing?

[FlorianWendelborn]

@jbucaran Sorry to quote myself again, but please re-read my last comment.

This issue isn't about the way I store the reference to an element, that's just an example. This issue is about hyperapp's events breaking when using hyperapp's own actions. There is no reason why this should break anything if I update the state. Instead, one solution would be to defer the next render until the current render is done.

[jorgebucaran]

@dodekeract Got it. Well, perhaps we need to rethink oncreate? But we can't break @zaceno's case either.

[FlorianWendelborn]

@jbucaran The issue isn't really that we can't allow state-changes, the issue is that hyperapp tries to render twice at once. We should defer any (state changed -> we need to re-render) renders until the current render is done.

[jorgebucaran]

Sounds like that is a different issue? https://github.com/hyperapp/hyperapp/issues/90

[FlorianWendelborn]

@jbucaran Not necessarily. That depends on the implementation. It's not inheritely a problem that has to be solved asynchronously. It's also possible to defer this in a synchronous fashion or even cancel the current render and start the one with the new state immediately.

[jorgebucaran]

@dodekeract I need a system to help me evaluate what is worth doing and what isn't. I don't want to belittle this issue, but seems like in order to implement what you are suggesting we'd need to move things around quite a bit.

Is this something you'd be interested in experimenting with?

[FlorianWendelborn]

@jbucaran No, I don't have time for OpenSource stuff right now. I'm working while being a full-time student.

[zaceno]

If I'm reading the issue correctly, then it's not even really about the oncreate-handler. It's about the fact that it is possible to make hyperapp start a new render before the first one is done.

You'd have the same problem if you just called a reducer inside the view function.

So the solution would be to prevent more than one rendering from being started at a time. The requestAnimationFrame-approach seems like a good way to go.

Wether it's worth it or not to do, I don't know, but I agree with @dodekeract that people probably expect to be able to call actions at any time without crashing hyperapp. So if we don't do anything about this, that should at least be made clear in the docs.

[jorgebucaran]

@zaceno You'd have the same problem if you just called a reducer inside the view function.

I don't think so. I use actions inside view functions all the time. I think we're talking about a totally different issue now.

If you debug trace @dodekeract's example, you'll eventually find that this problem only occurs before the very first render, because we don't have a reference to the oldNode, but are trying to render a newNode.

[jorgebucaran]

@zaceno ...people probably expect to be able to call actions at any time without crashing hyperapp.

This is my comment that addresses the cause of the problem.

[jorgebucaran]

Closing in favor of #218.

[zaceno]

Ah, so requestAnimationFrame wouldnt help - it breaks in the first render. Hmm...