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

ensure we don't compile typeof checks poorly #4960

Merged
merged 3 commits into from
May 3, 2017

Conversation

runspired
Copy link
Contributor

When doing a typeof comparison against anything other than a string, Babel compiles the check into a much more expensive function call. Some examples of this which are present in our code include:

const type = typeof thing;

and

switch (typeof Foo)

Example output:

var _typeof = typeof Symbol === "function" && typeof Symbol.iterator === "symbol" ? function (obj) { return typeof obj; } : function (obj) { return obj && typeof Symbol === "function" && obj.constructor === Symbol && obj !== Symbol.prototype ? "symbol" : typeof obj; };

var type = typeof thing === "undefined" ? "undefined" : _typeof(thing);

To deal with this, because we do not rely on iterator or Symbol in our code, I added transform-es2015-typeof-symbol to the list of excluded babel plugins.

Copy link
Member

@hjdivad hjdivad left a comment

Choose a reason for hiding this comment

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

LGTM (sans test failures ofc)

@runspired
Copy link
Contributor Author

@hjdivad test failures are unrelated, all PRs are failing. I spent some time digging in over the weekend and it looks to be because our yarn lock file installs a different set of dependencies locally than we get when npm installing in CI. I haven't totally unwound which dependencies need to be officially bumped in order to make the failures stop.

@stefanpenner
Copy link
Member

@runspired 👍

@bmac bmac merged commit 2caf05e into emberjs:master May 3, 2017
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