Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

window.document.body being null crashes ko disgracefully #1215

Closed
zpdDG4gta8XKpMCd opened this issue Nov 20, 2013 · 5 comments
Closed

window.document.body being null crashes ko disgracefully #1215

zpdDG4gta8XKpMCd opened this issue Nov 20, 2013 · 5 comments
Labels
affected: very few severity: minor type: bug
Milestone
3.5.0

Comments

@zpdDG4gta8XKpMCd
Copy link

My bad that instead of

function start() { ko.applyBindings({}); }
$(start);

I did

function start() { ko.applyBindings({}); }
$(start());

if you do it from the html/head the body isn't present yet and you get a null reference exception from (see a line below):

function applyBindingsToNodeAndDescendantsInternal (bindingContext, nodeVerified, bindingContextMayDifferFromDomParentElement) {
        var shouldBindDescendants = true;

        // Perf optimisation: Apply bindings only if...
        // (1) We need to store the binding context on this node (because it may differ from the DOM parent node's binding context)
        //     Note that we can't store binding contexts on non-elements (e.g., text nodes), as IE doesn't allow expando properties for those
        // (2) It might have bindings (e.g., it has a data-bind attribute, or it's a marker for a containerless template)
        var isElement = (nodeVerified.nodeType === 1); <--- *** HERE ***
/* ... */
}
@vamp
Copy link

vamp commented Nov 20, 2013

It's bad practice - to put your scripts in <head> section. In my opinion - It's your disgrace ;)

@brigand
Copy link

brigand commented Nov 20, 2013

I think it's kinda outside of the scope of Knockout. It should be specified in the docs if it isn't already, but I don't think a code change would be positive.

Most people do something like this if they include jQuery.

$(function(){
    ko.applyBindings(viewModel);
});

Personally, I put this in the bottom of my HTML file, so the order of files doesn't matter, and they can be loaded in the head, end of body, middle of body.. wherever.

var viewModel = new MyMainViewModel();
ko.applyBindings(viewModel);

@zpdDG4gta8XKpMCd
Copy link
Author

Everything you guys have said makes sense, my point is that KO needs an additional check for null and an error message that makes sense at applying bindings.

@mbest
Copy link
Member

mbest commented Jun 21, 2014

A simple code change, to move rootNode = rootNode || window.document.body before the code that checks rootNode could help with this. At least, then, you'd get a meaningful exception.

@mbest mbest added this to the 3.5.0 milestone Dec 2, 2016
@mbest mbest closed this as completed in d2a9e44 Dec 3, 2016
@LoganDark
Copy link

This got me for a while before I realized I wasn't waiting for onLoad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
affected: very few severity: minor type: bug
Projects
None yet
Milestone
3.5.0
Development

No branches or pull requests
6 participants
@vamp @SteveSanderson @brigand @zpdDG4gta8XKpMCd @mbest @LoganDark