-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[REGRESSION]: Difference between Ember 1.11.0 and Ember 1.11.0-beta.5 rendering/run loop #10880
Comments
I have a full ember-cli app to show this in action The tests I link to above show that previously (in 1.11-beta 5) I could run the loop and see the content loading before a promise was resolved Here is the basic template exercised in the test above if you are still looking for more context around how this did work/etc. {{#if controller.content.isLoaded}}
{{#if controller.content}}
<div class="found">show data here</div>
{{else}}
<div class="no-data-found"><h2>No data available</h2></div>
{{/if}}
{{else}}
<div class="loading-gif"><h2>Loading...</h2></div>
{{/if}} |
That's because there was a bug where some async helpers were triggered synchronously, and you were relying on this (accidental) behavior in your tests. One possible solution is to replace Ember.Test.registerHelper('syncVisit', function(app, url) {
// .. code to visit a route goes here ..
return app.testHelpers.wait();
}); The long term solution is to use ES7 async/await and make all test helpers sync. This will resolve the async helpers issue. See ember-cli/ember-cli#3529. |
I have good news and bad news. Good NewsThis isn't a regression! 😂 Bad NewsYou are kinda screwed. 😞 Seriously, though here is the lowdown on why it was working and now why it is not:
The first async helper was always being executed synchronously (in your case In your case here, the
In #10463 the bug causing the first helper to be ran synchronously was fixed (it really was a bug). Now all async helpers are always async. Unfortunately, this means that there is no way (that I know of) at the moment to test the loading states/templates without visiting them directly. |
@teddyzeenny - Thanks for explaining the way forward on this... |
@rwjblue correct me if I'm wrong but it looks like the syncVisit helper @teddyzeenny showed above would provide the ability to inspect the template for loading state/etc |
@toranb - Yep, it works but I don't really consider that a "good" solution for the long term (but it is great for now until the async/await stuff is ready for primetime). As an Ember app developer you generally shouldn't be forced to copy (and maintain as it changes) code from built-in test helpers. I'm mostly just saying that we need to do better here, and @teddyzeenny + @stefanpenner have been paving the way on the better long term solution (via the async/await stuff). |
Thank you for this great explanation! |
@williamsbdev can you close the issue as it's something we can "work around" and not a regression as we originally thought. @teddyzeenny one question about "how to" implement that sync based visit helper. Looking at the visit source it appears to "wait" (using 1.11.2 -looking at ember.debug.js from bower) function visit(app, url) {
var router = app.__container__.lookup('router:main');
router.location.setURL(url);
if (app._readinessDeferrals > 0) {
router['initialURL'] = url;
run['default'](app, 'advanceReadiness');
delete router['initialURL'];
} else {
run['default'](app.__deprecatedInstance__, 'handleURL', url);
}
return app.testHelpers.wait();
} I originally took your comment to mean "re implement visit but wait" so now I'm unsure of what this helper looks like (that would be different than visit above). Also, even when I added this test helper using the syntax you listed above - I get syncVisit is not defined when the tests run. Is an additional step required to "officially register" the helper so I can use it as easily as I did visit? |
@toranb it would be exactly the same as the original
You'll want to register it before calling |
First up -adding visitSync to my .jshintrc (for tests) eliminated the visitSync not found issue I mentioned above. (sorry for the newb like panic but it didn't mention jshint in the breakdown) Next - just hacking around in the ember source I added this line (as you suggested) and it solved the problem. Now I can do a simple Ember.run as I did in beta 5 :) asyncHelper('visit', visit);
helper('visitSync', visit); |
@teddyzeenny the very last question is - now that I'm trying to register this before startApp I need something that works and today the below isn't working to register this correctly. I assume it's because visit isn't a "thing" in beforeEach like it is inside the QUnit test block. What is the best way to register this before startApp (but in a way I can tap into visit to reduce pure duplication if that's an option) module('Acceptance: My Test Suite', {
beforeEach: function() {
Ember.Test.registerHelper('visitSync', visit);
application = startApp();
}
}); |
@toranb unfortunately you have to re-define the Note that you don't need to call |
@teddyzeenny you be up to help solve this some night in the next 2 weeks for an hourly bill rate? I've tried a few different approaches and cannot get around this jshint error saying "redefinition of visitSync" that I can't seem to find (did a complete fresh npm install / bower install - blew away tmp/dist/etc) If you are free for some consulting work ping me offline :) toranb at gmail Here is what I've tried (in the startApp helper) with no luck so far var visitSync = function(app, url) {
var router = app.__container__.lookup('router:main');
router.location.setURL(url);
if (app._readinessDeferrals > 0) {
router['initialURL'] = url;
Ember.run(app, 'advanceReadiness');
delete router['initialURL'];
} else {
Ember.run(app.__deprecatedInstance__, 'handleURL', url);
}
return app.testHelpers.wait();
};
Ember.Test.registerHelper("visitSync", visitSync); |
@toranb that's because you have (correctly) configured Ember.Test.registerHelper("visitSync", function(app, url) {
var router = app.__container__.lookup('router:main');
router.location.setURL(url);
if (app._readinessDeferrals > 0) {
router['initialURL'] = url;
Ember.run(app, 'advanceReadiness');
delete router['initialURL'];
} else {
Ember.run(app.__deprecatedInstance__, 'handleURL', url);
}
return app.testHelpers.wait();
}); This should solve the issue :) |
these is no point in this |
this should be considered a short-term solution. Please update your tests to not rely on this being sync, as it is a pretty dubious thing to rely on. |
@teddyzeenny haha that did the trick :) **now where to mail the check for all your help today @stefanpenner In my example app the Ember.run allowed me to verify the conditional in my hbs templates for loading state/empty state/loaded state. Without this (and the visit sync helper) I cannot do automated testing around the pre-loaded state **unless you can prove me wrong :) |
That run doesn't appear to be wrapping any run loop code. So it appears like a no opt. Or am I missing something |
I'm closing this issue as this is not a real bug and I believe we have a way to do what we need. Thank you everyone @teddyzeenny @rwjblue @stefanpenner @toranb! |
Sorry to bring this back up, but is there any chance that there is better support for this in the 2.x series of Ember. I can open a new issue if needed. Our goal is to functionally test that we correctly configured the loading route. @stefanpenner @rwjblue @teddyzeenny |
We have an app that is relying on the way that Ember is rendering/using the run loop in testing. We are trying to have the templates render a "Loading..." state before a promise returning the array of items has been resolved. In our test we are doing the following and this passes in Ember 1.11.0-beta.5 but fails in Ember 1.11.X:
We are returning an ArrayProxy that gets populated by the promise and we have a check in the template whether the ArrayProxy "isLoaded" as we are setting the "isLoaded" property on the ArrayProxy when the promise resolves.
The app works perfectly when running
ember server
when I have the same stubbed request and a response time of 2 seconds. I get the "Loading..." state in the template and then the array populates when the promise finally resolves and the data is shown.The text was updated successfully, but these errors were encountered: