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.

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

Cannot find closing comment tag to match: ko with: blog #11

Closed
jlaustill opened this issue Nov 22, 2016 · 13 comments
Closed

Cannot find closing comment tag to match: ko with: blog #11

jlaustill opened this issue Nov 22, 2016 · 13 comments

Comments

@jlaustill
Copy link

jlaustill commented Nov 22, 2016

Edit.tmpl.html.tar.gz
After fixing all my click bindings, I'm now getting this error. I'm attaching the view. This works in 3.4.0 and could very well be another thing I'm doing wrong and stupid, or a bug in tko. I went over the view and nothing popped out at my, maybe it will at you. Error and stack below, view attached.

ko.js:898 Uncaught Error: Cannot find closing comment tag to match:  ko with: blog 
    at getVirtualChildren (http://local.north40net.com/node_modules/tko/dist/ko.js:898:17)
    at getMatchingEndComment (http://local.north40net.com/node_modules/tko/dist/ko.js:903:32)
    at nextSibling (http://local.north40net.com/node_modules/tko/dist/ko.js:1013:18)
    at invokeForEachNodeInContinuousRange (http://local.north40net.com/node_modules/tko/dist/ko.js:6236:25)
    at activateBindingsOnContinuousNodeArray (http://local.north40net.com/node_modules/tko/dist/ko.js:6256:15)
    at executeTemplate (http://local.north40net.com/node_modules/tko/dist/ko.js:6331:11)
    at computed.disposeWhen (http://local.north40net.com/node_modules/tko/dist/ko.js:6373:44)
    at Function.evaluateImmediate_CallReadThenEndDependencyDetection (http://local.north40net.com/node_modules/tko/dist/ko.js:2667:105)
    at Function.evaluateImmediate_CallReadWithDependencyDetection (http://local.north40net.com/node_modules/tko/dist/ko.js:2639:31)
    at Function.evaluateImmediate (http://local.north40net.com/node_modules/tko/dist/ko.js:2603:20)
getVirtualChildren @ ko.js:898getMatchingEndComment @ ko.js:903nextSibling @ ko.js:1013invokeForEachNodeInContinuousRange @ ko.js:6236activateBindingsOnContinuousNodeArray @ ko.js:6256executeTemplate @ ko.js:6331computed.disposeWhen @ ko.js:6373evaluateImmediate_CallReadThenEndDependencyDetection @ ko.js:2667evaluateImmediate_CallReadWithDependencyDetection @ ko.js:2639evaluateImmediate @ ko.js:2603evaluatePossiblyAsync @ ko.js:2569notifySubscribers @ ko.js:1743valueHasMutated @ ko.js:1899Observable @ ko.js:1865(anonymous function) @ knockout-amd-helpers.js:189execCb @ require.js:1693check @ require.js:881(anonymous function) @ require.js:1136(anonymous function) @ require.js:134(anonymous function) @ require.js:1186each @ require.js:59emit @ require.js:1185check @ require.js:936enable @ require.js:1173init @ require.js:786(anonymous function) @ require.js:1011(anonymous function) @ require.js:134finishLoad @ text.js:169(anonymous function) @ text.js:205xhr.onreadystatechange @ text.js:317
@brianmhunt
Copy link
Member

Thanks for the report, @jlaustill — I had a look but the problem didn't stand out.

Would you mind setting up a jsFiddle so we can test it out?

@jlaustill
Copy link
Author

I can work on that, but it may end up being VERY complicated. I'm using knockout-amd-helpers to load my templates with requirejs-text. So the issue may end up being part of the loading process there. I'm not even sure how to mock that in a fiddle. So I'll start with just the template and see if the issue pops up. If it doesn't, I'll have to get creative to reproduce and find where the issue is. I'm betting after thinking on it for awhile that it has something to do with knockout-amd-helpers and the way it passes the template off.

Either way, working through this fiddle will be a learning experience and worth it.

@jlaustill
Copy link
Author

Fiddle

This was way easier to duplicate than I expected! Sorry it took so long. I ended up not needing anything but tko as a dependency to dupe, so hopefully that makes it easier to diagnose.

@brianmhunt
Copy link
Member

brianmhunt commented Nov 25, 2016

Good news (if this fixes the issue):

The closing /ko is not in an actual HTML comment -- the ! is missing. If you change it from <-- /ko --> to <!-- /ko --> then the parser will work.

(Instead of comment tags, you could also use {{ with: thing }}, and {{ /with }})

Let me know if that doesn't resolve the issue in your code.

@jlaustill
Copy link
Author

Well shoot, this isn't good lol. This means that I messed up in my fiddle what wasn't messed up in the original code :) And this indeed works, so I'm guessing it's an issue with knockout.amd.helpers not being tko compatible(which is kinda expected I think), or requirejs-text loading the template with a weird ending? I'll try the punches syntax when I get back to the office on monday and see how that works. Thanks for the tips, I'm actually learning a lot in the process of testing tko!

@jlaustill
Copy link
Author

jlaustill commented Nov 28, 2016

Ok, I tried the punches syntax. Same error. After some poking around I noticed that the comment/closing tag is being rendered BEFORE the closing form tag. So in my html I have

</form>
<!-- Well this is interesting! -->
{{/with}}

But in the Elements inspector in chrome dev tools, I end up with

<!-- Well this is interesting! -->
{{/with}}
</form>

Somehow the tags are getting reordered. I'm not sure how to debug this because I have so many pieces in play. The common denominator however is that it works with 3.4.0, so I just need to figure out what's changed in the way that templates are loaded through requirejs-text and knockout-amd-helpers vs having the template directly in the html, like in the fiddle.

I think I may end up having to build a blank project on github and start adding pieces to it until the issue presents itself, because I'm not sure that I can add requirejs and the text loader to a fiddle.

@brianmhunt
Copy link
Member

Thanks @jlaustill – it looks like a fairly involved investigation. I appreciate your help rooting out the cause.

@rniemeyer – There seems to be an issue with TKO and knockout-amd-helpers, where everything works with ko 3.4, but with tko there's some node/comment reversal happening (as described above).

I've rewritten a bunch of the templating code in tko.binding.template/src, which – although passing the KO unit tests – is quite likely the cause here.

I wanted to loop you in, just in case the cause is obvious or fix is apparent.

Thanks Ryan!

@rniemeyer
Copy link
Member

@brianmhunt @jlaustill - nothing obvious comes to mind, but I'd be happy to try to trouble-shoot a bit when I have a chance.

@jlaustill
Copy link
Author

Sorry this took so long, but I finally got around to doing up a basic repo to test this. tko QA This works with a simple template and the bug is not reproduced. Therefore, it may be a more complicated template that is causing it. I'll build on this when I have another moment and add a way more complex template like the one that is breaking in my main project. If that doesn't reproduce it, then it may be a conflict with another library that I'm using which will be way harder to track down because my project has a TON of them...

This is encouraging either way.

@brianmhunt
Copy link
Member

Thanks for the effort - I hope we can isolate it!

@jlaustill
Copy link
Author

jlaustill commented Dec 30, 2016

Ok, I just decided to make time. I was able to reproduce the issue. It appears to be caused by having a form inside the <!-- ko with.... syntax. Running the form works fine without the with syntax. And the with syntax works fine without the form. I updated the tkoQA repo so you can pull it and test it yourself. I included everything in the repo, no .gitignore. So you should be able to simply clone it and

npm run browser-sync

if I did everything right. You may need to have browser-sync installed globally for it to work, not sure.

@brianmhunt
Copy link
Member

Sorry for the delay getting back to this — what's the reproducibility of this in a jsFiddle?

I suspect if form is involved that there may be some hoisting of the comments (as we see with <table> tags etc.)

A jsFiddle will go a long way.

@brianmhunt
Copy link
Member

Some updates for tko alpha4 / beta may fix this, and I'll close this just to clean it up, but if the issue persists please comment and I'll reopen. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants