Hyperapp: Subscriptions are ignored when running app via <head> tag
Relevant code https://github.com/hyperapp/hyperapp/blob/master/app.js#L81.
Ping @dodekeract.
Resources
jorgebucaran
All 24 comments
Something like this would do.
ready(function () {
for (var sub in subs) {
subs[sub](model, msg, hooks.onError)
}
})
function ready(cb) {
document.readyState === "complete"
? cb()
: document.addEventListener("DOMContentLoaded", cb)
}
@dodekeract Is there any reason why we can't just treat "render app()" as the first sub?
Interesting question. That way you wouldn't even need to detect DOMContentLoaded yourself.
jorgebucaran
on 8 Feb 2017
They'll call us witches and burn us alive. See magic.
jorgebucaran
on 8 Feb 2017
Document ready, etc. Ugh! It's complicated. Here's what I came up with to deal with the mess. Handles situations where the document is already ready for different browsers since they can't decide on how to let us now this so unimportant and insignificant condition ;-P:
function ready(callback) {
const readyRE = /complete|loaded|interactive/;
if (readyRE.test(document.readyState) && document.body) {
callback.call(callback);
} else {
document.addEventListener("DOMContentLoaded", function() {
return callback.call(callback);
});
}
}
rbiggs
on 8 Feb 2017
I don't think it should be in the app/framework side. I believe it would be better to expose .start method or just such. So user will "start" the app when and where he want. That's why I exposed .start() in gibon router - before calling start it does not listen on any events.
tunnckoCore
on 8 Feb 2017
I agree with @tunnckoCore. Instead of a clunky .start() method, however, you already have a function, app, which you can call when you are ready.
Thanks @rbiggs! I'm almost done implementing this.
I'll push in a while with a few other minor improvements.
jorgebucaran
on 8 Feb 2017
@tunnckoCore Having a .start() method would just lead to my initial question once again. Which event should that be attached to so that subs don't break?
FlorianWendelborn
on 9 Feb 2017
Hi @dodekeract, this is now fixed in the master branch. Subscriptions (not subs anymore) will work even when you run app(...) inside DOMContentLoaded.
See https://github.com/hyperapp/hyperapp/blob/master/app.js#L8
jorgebucaran
on 9 Feb 2017
@jbucaran thanks, will open another issue if it doesn't work, but looks good.
FlorianWendelborn
on 9 Feb 2017
@dodekeract Thanks, I'll keep this open. Test it out and please close when, and if, the previous behavior is fixed.
jorgebucaran
on 9 Feb 2017
@jbucaran I can still reproduce this when using yarn add git+ssh://[email protected]/hyperapp/hyperapp.git. node_modules/hyperapp/app.js also seems to be the current version.
I updated dodekeract/hyperapp-issue-61 accordingly.
FlorianWendelborn
on 9 Feb 2017
@dodekeract You're using the wrong bundle in broken.html, it should be bundle.onload.js.
And as for broken.root.html, the correct code should be:
window.addEventListener('load', () => app({
view: () => html`
<div style=${{
position: 'fixed',
bottom: '8px',
right: '8px',
backgroundColor: 'yellow'
}}>
View
</div>`,
root: document.getElementById('hyperapp')
}))
| Not broken.html | Not broken.root.html |
|---|---|
| 
| 
|
@dodekeract
jorgebucaran
on 9 Feb 2017
@jbucaran Of course I'm using index.js in both index.html and broken.html. That's why one is called broken, because it doesn't reliably work when included in the head without an event listener.
Only thing I'll change is that onload.html should actually have the script in the head, but that doesn't make any difference.
index.js -> index.html + broken.html
index.root.js -> broken.root.html
index.onload.js -> onload.html
Working: index.html, onload.html
Broken: broken.html, broken.root.html
FlorianWendelborn
on 9 Feb 2017
@dodekeract Why?
jorgebucaran
on 9 Feb 2017
@jbucaran Because the repo demonstrates the different ways to include hyperapp and every html called broken is a demo of a way that doesn't work?
FlorianWendelborn
on 9 Feb 2017
In that case, the title of this issue is misleading (or rather this issue is now fixed, we should open a new one). Subscriptions do work now when running the app via <head>.
jorgebucaran
on 9 Feb 2017
@jbucaran It still crashes in both broken.html and broken.root.html.
FlorianWendelborn
on 9 Feb 2017
@dodekeract Are you using window.addEventListener("load") or document.addEventListener("DOMContentLoaded") to run the app?
jorgebucaran
on 9 Feb 2017
@jbucaran In onload.html I'm using that, yes. That's the point of having multiple slightly different files with different ways to include/call hyperapp.
Is it required now to use window.addEventListener to make this work?
EDIT: I added subscriptions: [_ => alert(1)] to the repo. Seems that index.html and onload.html now work (also execute subs). Other two still crash though.
FlorianWendelborn
on 9 Feb 2017
@dodekeract We now understand each other.
At the moment, yes, it's required because when embedding hyperapp in the <head>, hyperapp's render/patch functions will fail, since the document is not ready yet.
The question is, should we make hyperapp aware of this out of the box? and why?
@tunnckoCore @terkelg @tzellman
jorgebucaran
on 9 Feb 2017
@jbucaran No matter what the decision is, the behavior should be documented. 馃檪
FlorianWendelborn
on 9 Feb 2017
@jbucaran Issue can be closed then. I'd suggest to open a new one with docs/question if this should work without window.addEventListener or not.
Thanks. That's everything I need for the use-case.
FlorianWendelborn
on 9 Feb 2017
@dodekeract That sounds like the right thing to do. Can you take care of opening it?
I'll close here then.
jorgebucaran
on 9 Feb 2017
Related issues
jorgebucaran
路
4Comments
icylace
路
3Comments
jscriptcoder
路
4Comments
ghost
路
3Comments
dmitrykurmanov
路
3Comments