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

EJS renders "@@!!@@" instead of Model data when a Deferred is passed into can.view that takes a long time to resolve #230

Closed
colinexl opened this Issue Jan 10, 2013 · 6 comments

Comments

Projects
None yet
3 participants
@colinexl

colinexl commented Jan 10, 2013

EJS renders "@@!!@@" instead of Model data when the following conditions are satisfied:

  1. Can.view takes at least one Deferred
  2. Can.view has a callback function defined
  3. A Deferred takes a "long" time to resolve

The Can.view Deferred will get resolved, which calls pipe() to generate the DOM fragment. The side effect of pipe() is that it cleans up after itself after elements have been hooked up with data.

However, if a callback function is defined, it calls pipe() a second time which doesn't hook up any data because the first pipe() already cleaned up after itself.

As a result, "@@!!@@" stays in the DOM fragment.

The work around is to not rely on a callback function in the Can.view and use .then() after Can.view.

For example:

can.view('view', {Deferred objs}).then(function(fragment) { render fragment });

NOT:

can.view('view', {Deferred objs}, function(fragment) { render fragment });

The fix would be to call the callback function with the proper DOM fragment after the view Deferred gets resolved.

Thanks,
Colin Zhu

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Jan 11, 2013

Contributor

Are you using the latest 1.1.3? I remember that a fix for something like this (can.view callback being piped in the Deferred) made it in there.

If you do, could you provide a Fiddle that demonstrates your issue? @@!!@@ is not the usual live-binding placeholder. I think it has something to do with binding tables.

Contributor

daffl commented Jan 11, 2013

Are you using the latest 1.1.3? I remember that a fix for something like this (can.view callback being piped in the Deferred) made it in there.

If you do, could you provide a Fiddle that demonstrates your issue? @@!!@@ is not the usual live-binding placeholder. I think it has something to do with binding tables.

@ms

This comment has been minimized.

Show comment
Hide comment
@ms

ms Jan 11, 2013

Contributor

(Same team here)

Actually we can't reproduce it with 1.1.3, but we are currently using 689ef73. I don't know how to create a fiddle that'll steal that properly without specifying every single can file as a resource.

Contributor

ms commented Jan 11, 2013

(Same team here)

Actually we can't reproduce it with 1.1.3, but we are currently using 689ef73. I don't know how to create a fiddle that'll steal that properly without specifying every single can file as a resource.

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Jan 11, 2013

Contributor

Oh so that issue has been introduced after 1.1.3? Could you check if it has something to do with 0ddf3ea?

We don't deploy an edge CanJS build at the moment, so pasting the breaking code here in the comments is fine.

Contributor

daffl commented Jan 11, 2013

Oh so that issue has been introduced after 1.1.3? Could you check if it has something to do with 0ddf3ea?

We don't deploy an edge CanJS build at the moment, so pasting the breaking code here in the comments is fine.

@ms

This comment has been minimized.

Show comment
Hide comment
@ms

ms Jan 14, 2013

Contributor

It's definitely linked to 0ddf3ea.

On line 38, after the view.render deferred is resolved, the result is piped which resolves the hookups. But wrapCallback calls pipe on the result again after that, but by then there are no $view.hookups and therefore no !!@@!! get replaced.

If the deferred is resolved first, the callback will get incorrect data, and vice versa. Currently it seems that the deferred is resolved first (lines 377 and 380) so the callback gets incorrect results.

A simple minimal example that should break (I haven't tested it but I will if you need me to) is at this fiddle: http://jsfiddle.net/ePc8T/ (it doesn't here because 1.1.3 is not broken). Here is the code:

can.view.ejs('view_deferred', '<%= value %>');

var def = new can.Deferred();
setTimeout(function () {
  def.resolve('Hello can');
}, 1000);

can.view('view_deferred', {
  value: def
}, function (fragment) {
  $('#out-cb').html(fragment); // Incorrect frag with !!@@!!
}).then(function(fragment) {
  $('#out-then').html(fragment);
})
Contributor

ms commented Jan 14, 2013

It's definitely linked to 0ddf3ea.

On line 38, after the view.render deferred is resolved, the result is piped which resolves the hookups. But wrapCallback calls pipe on the result again after that, but by then there are no $view.hookups and therefore no !!@@!! get replaced.

If the deferred is resolved first, the callback will get incorrect data, and vice versa. Currently it seems that the deferred is resolved first (lines 377 and 380) so the callback gets incorrect results.

A simple minimal example that should break (I haven't tested it but I will if you need me to) is at this fiddle: http://jsfiddle.net/ePc8T/ (it doesn't here because 1.1.3 is not broken). Here is the code:

can.view.ejs('view_deferred', '<%= value %>');

var def = new can.Deferred();
setTimeout(function () {
  def.resolve('Hello can');
}, 1000);

can.view('view_deferred', {
  value: def
}, function (fragment) {
  $('#out-cb').html(fragment); // Incorrect frag with !!@@!!
}).then(function(fragment) {
  $('#out-then').html(fragment);
})

@ghost ghost assigned daffl Jan 14, 2013

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Jan 23, 2013

Contributor

Ok, I tried putting this into a test and either this issue is fixed in the latest master or I'm doing something wrong to reproduce it. Here is the tests code I am using which passes on the current master:

stop();
can.view.ejs('view_deferred', '<%= value %>');

var def = new can.Deferred();
setTimeout(function () {
    def.resolve('Hello can');
}, 1000);

$('#qunit-test-area').append('<div id="out-cb"></div><div id="out-then"></div>')

can.view('view_deferred', {
    value: def
}, function (fragment) {
    equal($('#out-cb').html(fragment).html(), 'Hello can', 'callback rendered content');
}).then(function(fragment) {
    equal($('#out-then').html(fragment).html(), 'Hello can', '.then rendered content');
    start();
});
Contributor

daffl commented Jan 23, 2013

Ok, I tried putting this into a test and either this issue is fixed in the latest master or I'm doing something wrong to reproduce it. Here is the tests code I am using which passes on the current master:

stop();
can.view.ejs('view_deferred', '<%= value %>');

var def = new can.Deferred();
setTimeout(function () {
    def.resolve('Hello can');
}, 1000);

$('#qunit-test-area').append('<div id="out-cb"></div><div id="out-then"></div>')

can.view('view_deferred', {
    value: def
}, function (fragment) {
    equal($('#out-cb').html(fragment).html(), 'Hello can', 'callback rendered content');
}).then(function(fragment) {
    equal($('#out-then').html(fragment).html(), 'Hello can', '.then rendered content');
    start();
});
@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Jan 23, 2013

Contributor

As mentioned, I can't reproduce it on the current master (even with IE, what the !!@@!! placeholder is mainly used for). I will close this issue for now as I can't do much more without a breaking test. Feel free to reopen with a testcase like the one above that breaks on current master.

Contributor

daffl commented Jan 23, 2013

As mentioned, I can't reproduce it on the current master (even with IE, what the !!@@!! placeholder is mainly used for). I will close this issue for now as I can't do much more without a breaking test. Feel free to reopen with a testcase like the one above that breaks on current master.

@daffl daffl closed this Jan 23, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment