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

Better failure detection #78

Closed
wants to merge 4 commits into from
Closed

Better failure detection #78

wants to merge 4 commits into from

Conversation

tomdale
Copy link
Contributor

@tomdale tomdale commented Dec 17, 2015

This commit wraps each file that gets put in vendor in a try/catch. This allows us to capture FastBoot-incompatible addons and print a much more useful error message in the console.

Note that this relies on monkeypatching Ember CLI's EmberApp method in a very hacky way. @stefanpenner thinks it may be appropriate to expose a hook that we can use instead.

@rwjblue
Copy link
Member

rwjblue commented Dec 18, 2015

Doesn't this break source maps?

@rwjblue
Copy link
Member

rwjblue commented Dec 18, 2015

I am also wondering about performance. It is my understanding that using a try catch in some cases prevent V8 from optimizing. Is that an issue here?

@stefanpenner
Copy link
Contributor

Doesn't this break source maps?

Yes it will, we will have to come up with a better strategy at some point. That being said, if this is only for fast-boot builds it may be ok for awhile?

@stefanpenner
Copy link
Contributor

@rwjblue it would prevent the context from being optimized, but not contexts contained within. I suspect this could affect access time of variables provided from the context with the try/catches. I don't know if we already do that in hot-paths... I am unsure as to the magnitude of the issue, some test may need to be done to see?

There is an alternative way for us to do this without try/catch, if it turns out to be a measurable issue.

function checkDeps() {
setTimeout(function() { /* check which deps have run and which haven't figuring out what went sideways */ });
}

startScript('jQuery');
// embed jQuery
endScript('jQuery');

@tomdale
Copy link
Contributor Author

tomdale commented Feb 24, 2016

I think ember-fastboot/fastboot#16 is a better solution to this. Let's revisit if this remains a problem for people.

@tomdale tomdale closed this Feb 24, 2016
@danmcclain danmcclain deleted the better-failure-detection branch May 11, 2016 21:07
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

Successfully merging this pull request may close these issues.

3 participants