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

[BUGFIX beta] Refactor ember-debug to support better prod stripping. #15224

Merged
merged 1 commit into from
May 11, 2017

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented May 11, 2017

Prior to this change, things like Ember.assert and Ember.runInDebug were still firing in production builds. Lots of debug specific code was also left in the minified builds.

Prior to this change, things like `Ember.assert` and `Ember.runInDebug`
were still firing in production builds.
@rwjblue
Copy link
Member Author

rwjblue commented May 11, 2017

Much easier to review this via: https://github.com/emberjs/ember.js/pull/15224/files?w=1

@rwjblue
Copy link
Member Author

rwjblue commented May 11, 2017

Here is a zip file of the assets (debug + prod + min) if folks want to review those too:

pr-15224-dist.zip

*/
deprecate = function deprecate(message, test, options) {
if (!options || (!options.id && !options.until)) {
deprecate(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this cause a stack overflow?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a similar thought, but no because we are passing options and options.id and options.until. If we remove the guarding if (and move those conditions to the deprecate's predicate) it would be a stack overflow.

@rwjblue rwjblue merged commit 5a35da5 into emberjs:master May 11, 2017
@rwjblue rwjblue deleted the fix-debug branch May 11, 2017 18:31
@lukemelia
Copy link
Member

@rwjblue One of our internal addons just started failing in beta and canary because Ember.deprecate is undefined. Maybe related to this change?

@rwjblue
Copy link
Member Author

rwjblue commented May 12, 2017

Confirm. Thank you for spotting and reporting so quickly.

@Turbo87
Copy link
Member

Turbo87 commented May 12, 2017

@rwjblue I've hit the same issue in ember-cli-shims just now

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.

4 participants