Uncaught Error: Can't create second landmark in same branch #281

Closed
joshrtay opened this Issue Aug 18, 2012 · 38 comments

Projects

None yet

10 participants

@joshrtay

Getting this error a fair amount when using spark.

What does it mean and how do I setup my templates to avoid it?

@joshrtay

It seems that the issues is that the triple stache isn't given a label.

I was able to resolve the issue by changing the else if block at evaluate.js:315 as follows:

replace

// {{{triple stache}}}
buf.push(toString(invoke(stack, elt[1])));

with

// {{{triple stache}}}
var branch = elt[1]+"@"+getPCKey(index);
var html = Spark.labelBranch(branch,function() {
   return toString(invoke(stack, elt[1] || ''));
});
buf.push(html);
@gschmidt
Member

Nice catch. Hmm.

  1. The reason we don't label {{{ is that labels must fall on DOM node boundaries. We wanted to support <div {{{attributes}}}>.

  2. But it's unacceptable that you did something that seemed reasonable, and got a confusing exception.

Can you tell me a little more about how your templates are set up? Are you possibly doing an #each over an array of items that don't have _id attributes? Do you have two {{{ }}} side by side that both render other templates to compute their return value? Does the code for your {{{ }}} helper render two different templates (or the same template twice?) I'd like to understand exactly what's producing the label conflict.

@joshrtay

It is two {{{ }}} within the same div that render the same template to compute their return value.

I'm working on making a uikit with spark, so i created a helper called selector that uses the options passed into it to render a selector. When I put two selectors into the same div I seem to get errors on re-renders. If i could access the appropriate label from within the selector helper I could just call labelBranch myself.

@joshrtay

Another solution would be to allow for keyword arguments in partials. Then I wouldn't have to use the triple stache to render a template.

@dgreensp

The kind of solution we had in mind for this would be to have the helper assign a unique label to the template invocation, perhaps derived from the helper arguments.

However, I'm implementing a fix right now. All helpers will get labelBranch, which will be changed so that it only emits an annotation if it contains a landmark. This will make it work in your case and also support Geoff's case (1).

@joshrtay

Looks good. Thanks.

@dgreensp

Ok, now all helpers come with a label! This also means you can return a raw landmark from a non-block helper.

If you want to call two or more templates or landmarks from the same helper, you still need to wrap them in different labels.

Thanks for the feedback.

@dgreensp dgreensp closed this Aug 22, 2012
@gschmidt
Member

David, you know, as long as we're labeling everything, we could label each template invocation too (with the name of the template.) That would allow you to call two templates from the same helper as long as they were different templates.

Or should we go ahead and push 0.4.0 and see if anyone runs into this in practice?

@dgreensp

Good idea, I think it's worth the extra LiveRange. It will also cause the two templates in return (test ? Template.foo() : Template.bar()) not to match each other, which I think is what you'd expect. (In fact, it will force them not to match.)

@tmeasday tmeasday referenced this issue Sep 10, 2012
Closed

Model support #267

@tmeasday
Member

Thought you guys might want to know I'm seeing this when doing {{#each}} on a cursor that returns models, (as opposed to POJOs that contain an _id attribute).

Obviously this isn't really your problem, but I thought it might be interesting to know.

@tmeasday
Member

Looking slightly closer at this; @dgreensp - I wonder if you should have hit https://github.com/meteor/meteor/blob/master/packages/templating/deftemplate.js#L20 in this commit c149a10.

If I change the null to Spark.UNIQUE_LABEL on that line, the error goes away. I won't pretend I really know what's going on, but it's probably worth taking a look at.

@dgreensp

Tom: Sounds exactly right. I'll commit the change on devel. Thanks!

@tmeasday tmeasday added a commit to tmeasday/meteor that referenced this issue Sep 11, 2012
@dgreensp dgreensp Fall back to UNIQUE_LABEL on {{#each cursor}} (#281) 2f544e9
@tmeasday
Member

Awesome David, worked. Thanks.

@mb21
mb21 commented Feb 5, 2013

Hey guys, I've got the same problem: http://jsonedit.meteor.com (I assume you've got my source code there?)
Simply click on the second list item, then on the first item to provoke the error.

@dgreensp
dgreensp commented Feb 5, 2013

I'd love to take a look. We don't have easy access to the source. Can you
post or link to it?

On Tue, Feb 5, 2013 at 9:28 AM, mb21 notifications@github.com wrote:

Hey guys, I've got the same problem: http://jsonedit.meteor.com (I assume
you've got my source code there?)
Simply click on the second list item, then on the first item to provoke
the error.


Reply to this email directly or view it on GitHubhttps://github.com/meteor/meteor/issues/281#issuecomment-13140981.

@mb21
mb21 commented Feb 5, 2013

Thanks! Here's the source on dropbox.

@dgreensp
dgreensp commented Feb 6, 2013

When a helper invokes a template or a block multiple times, it needs to label them. In key_value, do this:

Handlebars.registerHelper("key_value", function(obj, fn) {
    var buffer = "",
        key;

    for (key in obj) {
        if (obj.hasOwnProperty(key)) {
          buffer += Spark.labelBranch(key, function () { // added labelBranch
            return fn({key: key, value: obj[key]});
          });
        }
    }

    return buffer;
});
@mb21
mb21 commented Feb 6, 2013

Thanks a lot, now it works! Guess I'll have to read ALL of the documentation one day ;)

@kenyee
kenyee commented Apr 8, 2013

I think I have a variant of this...sorry about the ugly stack trace.
I did hack up the patch code in Chrome to stop it to look at it...see attached image...
PatchNodeNullBug

Exception from Deps recompute: TypeError: Cannot read property 'nodeName' of null
at [object Object].match (http://localhost:3000/packages/spark/patch.js?299cb25985c501315fbd758353d7f498697a39c3:237:12)
at http://localhost:3000/packages/spark/patch.js?299cb25985c501315fbd758353d7f498697a39c3:50:23
at http://localhost:3000/packages/spark/patch.js?299cb25985c501315fbd758353d7f498697a39c3:17:11
at http://localhost:3000/packages/spark/patch.js?299cb25985c501315fbd758353d7f498697a39c3:18:9
at http://localhost:3000/packages/spark/patch.js?299cb25985c501315fbd758353d7f498697a39c3:18:9
at http://localhost:3000/packages/spark/patch.js?299cb25985c501315fbd758353d7f498697a39c3:18:9
at http://localhost:3000/packages/spark/patch.js?299cb25985c501315fbd758353d7f498697a39c3:18:9
at http://localhost:3000/packages/spark/patch.js?299cb25985c501315fbd758353d7f498697a39c3:18:9
at Object._patch (http://localhost:3000/packages/spark/patch.js?299cb25985c501315fbd758353d7f498697a39c3:31:3)
at http://localhost:3000/packages/spark/spark.js?45c746f38023ceb80745f4b4280457e15f058bbc:638:13
logging.js:40Exception from Deps recompute: TypeError: Cannot read property '0' of undefined
at [object Object]._removeEntries (http://localhost:3000/packages/liverange/liverange.js?31c1f5eb15cbaa59c35a1a90a6506e7ef5c8f835:352:31)
at [object Object].insertBefore (http://localhost:3000/packages/liverange/liverange.js?31c1f5eb15cbaa59c35a1a90a6506e7ef5c8f835:549:29)
at [object Object].computePreservations (http://localhost:3000/packages/spark/spark.js?45c746f38023ceb80745f4b4280457e15f058bbc:497:19)
at Object.renderToRange (http://localhost:3000/packages/spark/spark.js?45c746f38023ceb80745f4b4280457e15f058bbc:625:26)
at [object Object]._func (http://localhost:3000/packages/spark/spark.js?45c746f38023ceb80745f4b4280457e15f058bbc:864:13)
at [object Object]._compute (http://localhost:3000/packages/deps/deps.js?9642a93ae1f8ffa8eb1c2475b198c764f183d693:129:12)
at [object Object]._recompute (http://localhost:3000/packages/deps/deps.js?9642a93ae1f8ffa8eb1c2475b198c764f183d693:142:14)
at http://localhost:3000/packages/deps/deps.js?9642a93ae1f8ffa8eb1c2475b198c764f183d693:224:14
logging.js:40Exception from Deps recompute: Error: Can't create second landmark in same branch
at Object.createLandmark (http://localhost:3000/packages/spark/spark.js?45c746f38023ceb80745f4b4280457e15f058bbc:1170:13)
at http://localhost:3000/packages/templating/deftemplate.js?b622653d121262e50a80be772bf5b1e55ab33881:116:24
at Object.labelBranch (http://localhost:3000/packages/spark/spark.js?45c746f38023ceb80745f4b4280457e15f058bbc:1115:14)
at Object.gallerythumb (http://localhost:3000/packages/templating/deftemplate.js?b622653d121262e50a80be772bf5b1e55ab33881:115:22)
at http://localhost:3000/packages/handlebars/evaluate.js?5f96328b2393e97e3f37d7fd00456ce6698fb895:352:47
at Object.labelBranch (http://localhost:3000/packages/spark/spark.js?45c746f38023ceb80745f4b4280457e15f058bbc:1115:14)
at http://localhost:3000/packages/handlebars/evaluate.js?5f96328b2393e97e3f37d7fd00456ce6698fb895:311:20
at http://localhost:3000/packages/handlebars/evaluate.js?5f96328b2393e97e3f37d7fd00456ce6698fb895:351:20
at Array.forEach (native)
at Function. (http://localhost:3000/packages/underscore/underscore.js?867d3653d53e9c7a171483edbcad9670e12288c7:79:11)

@kenyee
kenyee commented Apr 8, 2013

And FWIW, I changed that debug line to "return false" in Chrome and it seems to work fine (no crash in spark/patch)

@kenyee
kenyee commented Apr 9, 2013

Narrowed it down to these {{constants}}

<template name="gallerythumb">
      <li class="spanU">
        <div class="thumbnail">

{{#constant}}
          <a href="{{iconurl}}" data-pirobox="single" class="pirobox" 
title="{{filename}}">
<img class="lazy" data-src="{{iconurl}}" alt="{{filename}}" src="/images/grey.gif" width="200" height="200">
</a>
          <div>{{title}}</div>
{{/constant}}

        </div>
      </li>
</template>

I added the {{constant}} to try to cut back on the flicker from the reactivity. Apparently, this made spark freak out...this template is called inside a

    tag and then a {{#each}}. If I take out the {{constant}} lines, it works fine and doesn't need that spark change I mentioned above...

@dgreensp

@kenyee, thanks for this report. Is your #each over a plain array rather than a database cursor? If so, I think the problem is that we don't label the iterations of such a loop (probably we should!).

To confirm that this is the problem (and as a workaround), try defining a labelBranch block helper:

Handlebars.registerHelper('labelBranch', function (label, options) {
  var data = this;
  return Spark.labelBranch(label, function () {
    return options.fn(data);
  });
});

Then nest this immediately inside the #each, passing it some value that uniquely identifiers the iteration:

{{#each things}}
  {{#labelBranch id}}
    {{> renderThing}}
  {{/labelBranch}}
{{/each}}

The first error, "Can't create second landmark..." interrupts Spark at an awkward time and causes the other errors.

Let me know if this works!

@kenyee
kenyee commented Apr 11, 2013

If I do this:

        {{#each photos}}
{{#labelBranch _id}}
{{>gallerythumb}}
{{/labelBranch}}
        {{/each}}

None of the gallerythumb template parts display at all :-(

On the bright side, there aren't any "landmark" errors :-)

The data is from a mongodb collection...that's why I used _id there.

@dgreensp

Hmm. I'll probably need access to the code or some reproduction to debug further.

When you say the data is from a mongodb collection, it's significant whether it's the direct result of a find(...), which is a Cursor object, or the result of a fetch(...) or other method that returns an array.

@kenyee
kenyee commented Apr 11, 2013

It's from a direct result of a find, so definitely a Cursor object.

Would it help if I turned on --debug when deploying to choosepix.meteor.com? That's how I debugged it a bit (Chromium's debugger on Linux is flaky as hell so I had to use a Windows machine running Chrome).
It's up on your site already, so you should have access to the code...

The code currently on that test site is the fixed version w/o the #constant section.

@dgreensp

We don't have an easy way to access the source of a deployed app, which would be the built and obfuscated source anyway, at least for the client side and templates.

If it's a Cursor object, that kills my theory that the iterations of the #each aren't being labeled properly.

Are you ever calling templates from helpers and returning them?

@kenyee
kenyee commented Apr 11, 2013

If you do "meteor deploy --debug", the unobfuscated source is pushed up. That's how I was debugging it in Chrome :-)
I figured if you had access to it, you could swap in your own meteor code on the site for testing. Seems like the simplest way to do this because if I give you the code, you have to set it up on your system which is a bit of a pain w/ the google oauth stuff.

Nothing fancy in the helpers...didn't even know you could call templates from helpers (I've never seen an example of this).

The other thing is, I think it's some sort of race condition. I'm not sure if you've tried the app yet, but users can basically filter on a 5 star rating. When they change the star rating, it's a reactive find on the Photo collection on the client. On initial display, it's fine...it's only when you do a sequence of 0-star, 1+ star (doesn't matter which), then 0-star that the spark/landmark crash happens. It also doesn't happen on smaller collections of photos. I can make it happen at will on 100+ photo collection, but can't on a 6 photo collection.

@dgreensp

Ok, if you could deploy the app in debug mode, and in a state where the error can occur, that would be a great help. I can read the templates from the syntax tree, though I can't easily modify them. The proper way to track down something like this would be to start reducing the app, though if it's a race condition that depends on having a lot of data I could see that might be hard. My preferred way to debug this is still to be able to run the code.

If you can reproduce the error reliably enough, see if it has anything to do with jquery plugins. People are sometimes surprised at poor interactions between jquery and spark.

@kenyee
kenyee commented Apr 11, 2013

ok...it's up on choosepix.meteor.com. To reproduce, you need a public photo album on Google+ photos w/ a lot of images. Click on the add gallery button and it should show up, then you an invite yourself so you can rate images and then you can click on the rating filter button. Or if you let me know what your gmail account is, I can invite you to an album I have.

I can try to reduce the code down, but the problem is it depends on sucking photos off Google, so I can only get it down so far. It uses quite a few jquery plugins for displaying the gallery (and the plugins manipulate the DOM of the image code), so it could very well be that it's the problem....but even if it is, spark shouldn't crash out like this.

@dgreensp

@kenyee, I probably won't be able to take the time to track this down based on using the app. I believe it has to do with modifications to the DOM by jquery plugins. An upcoming version of Meteor will have ways of building UI that manipulate the DOM in ways that are easier to understand and integrate.

@kenyee
kenyee commented Apr 13, 2013

Looking forward to seeing what you guys come up w/ for the .rendered replacement. It is pretty weird for Meteor to change one UI element and then have it call .rendered multiple times even though it's reading out of minimongo locally.
And FWIW, I tried commenting out my jquery plugins one by one. I couldn't get rid of the jquery UI element that lets me change the filter, but I could remove all the others and then I'd see only square grey tiles instead of images. The problem still happens so I don't think it's the jquery plugins modifying the DOM. So my theory is that the multiple .rendered events for the same session.set reactive event are confusing spark when it generates the landmark labels. It can probably be reproduced by using a similar structure for templates and {{#constant}} w/ a dummy collection to filter on.

@funkjunky

I'm getting that same landmark error and I would simply like to suggest adding comments near the error explaining what all that terminology means. How am I supposed to know what the heck landmark, or branch mean in "Can't create second landmark in same branch"?

Issues are going to keep coming up, as they do in all projects, so I'm sure you will have lots more related to this error.

EDIT:
Ok, so I implemented the labelBranch hack and it fixed my problem. I'm wondering if this well be fixed in the next release version, because it is pretty silly that I either have to wrap my partial in a boilderplate helper, or not have the help, defeating the purpose of partials.

@TimHeckel

Can anyone comment if the 'labelBranch' hack for non-cursor (plain arrays) made it into the latest Meteor 0.6.6.1 ? I understand this is irrelevant when Meteor UI hits...

@avital
avital commented Oct 16, 2013

@TimHeckel No relevant change in 0.6.6.1, and you're right that this will be resolved with the new template engine, planned to be released as 0.7.0.

I think @dgreensp's comment above about explicitly labeling branches when using {{#each}} on non-cursors should resolve these issues. Have you tried that?

@TimHeckel

I haven't yet -- we have a pretty complex nested set of nested ../../ blocks in our template so adding labelBranch makes it a bit painful to reconfigure...can I add an id property to the array itself that would ensure Spark stitched a label onto it while it iterates? Or is the only way to hijack the process with the helper as noted above?

@avital
avital commented Oct 16, 2013

Yes, iterating over an array of objects containing _id fields should achieve the same result.

@TimHeckel

Great thanks -- what I've also found is giving the parent level container an idas well as all of its children in my markup works as well...at least it seems to have worked thus far.

@jzellis
jzellis commented Nov 18, 2013

I found another working solution here -- no idea why it works, but it works.

@dgreensp's labelBranch function failed for me, until I replaced 'label' with Spark.UNIQUE_LABEL, like so:

Handlebars.registerHelper('labelBranch', function (label, options) {
  var data = this;
  return Spark.labelBranch(Spark.UNIQUE_LABEL, function () {
    return options.fn(data);
  });
});

Then it worked like a charm. It was just a guess based on the Meteor wiki entry for Spark, but it seems to solve the problem, at least for me.

@musically-ut musically-ut added a commit to musically-ut/chanslate that referenced this issue Jan 19, 2014
@musically-ut musically-ut Remove the `foreach` helper to avoid a bug. 7e67f49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment