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

Fix issue with ember-lodash when using Babel polyfill on Phantom. #104

Merged
merged 1 commit into from
May 13, 2017

Conversation

rwjblue
Copy link
Contributor

@rwjblue rwjblue commented May 12, 2017

When lodash detects that Object.defineProperty has been modified by core-js polyfills, it intentionally exports undefined as its default export (the rest of lodash knows how to handle this).

However, when loader.js detects that exports.default === undefined it automatically sets exports.default = exports (which is mega WAT).

This causes the rest of the detection system that lodash has in place around its _defineProperty exporting undefined as default to completely fall apart. Since it expects that module to either export a function or undefined, when it sees that _defineProperty.default is truthy it simply invokes (what it thinks is a function). Due to the behavior of loader.js, this means that we are essentially doing:

let sadface = {};
sadface();

Which, low and behold, when ran on Phantom emits the following error:

Object is not a constructor (evaluating '(0, _lodash_defineProperty['default'])')

Thanks to @rondale-sc for helping me debug this.

Fixes #103
Fixes ember-cli/ember-cli#7001
Fixes miragejs/ember-cli-mirage#1084

When lodash detects that `Object.defineProperty` has been modified
by `core-js` polyfills, it intentionally exports `undefined` as its
default export (the rest of `lodash` knows how to handle this).

However, when `loader.js` detects that `exports.default === undefined`
it automatically sets `exports.default = exports` (which is mega WAT).

This causes the rest of the detection system that `lodash` has in place
around its `_defineProperty` exporting `undefined` as default to completely
fall apart. Since it expects that module to either export a function or
`undefined`, when it sees that `_defineProperty.default` is truthy it
simply invokes (what it thinks is a function).  Due to the behavior of
`loader.js`, this means that we are essentially doing:

```js
let sadface = {};
sadface();
```

Which, low and behold, when ran on Phantom emits the following error:

```
Object is not a constructor (evaluating '(0, _lodash_defineProperty['default'])')
```

Thanks to @rondale-sc for helping me debug this.
@rwjblue
Copy link
Contributor Author

rwjblue commented May 12, 2017

Note, I used broccoli-debug to make it much simpler to test the state of the modules during these modifications. There is no cost, when the BROCCOLI_DEBUG environment flag is not set.

I wrote a blog post about this a few days ago for those that are curious: http://rwjblue.com/2017/05/09/broccoli-debugging/.

@rwjblue
Copy link
Contributor Author

rwjblue commented May 12, 2017

Crosslinking upstream issue in loader.js to remove this automatic default export creation: ember-cli/loader.js#114.

@tchak
Copy link
Collaborator

tchak commented May 13, 2017

Thank you! ❤️

@tchak tchak merged commit f9f8678 into mike-north:master May 13, 2017
@rwjblue rwjblue deleted the fix-lodash-plus-phantom-plus-ouch branch May 13, 2017 05:07
@fotinakis
Copy link

Thanks @rwjblue! @tchak are you able to do a patch release to get this out?

@bsaff
Copy link

bsaff commented May 15, 2017

Nice work 👍

lennyburdette pushed a commit to lennyburdette/loader.js that referenced this pull request Jun 23, 2017
…lt` exports

If a module does not define a default export, loader.js assigns `exports.default = exports` for compatibility with modules that don't use `_interopRequireDefault` from babel6. Mutating exports is unexpected behavior and causes difficult-to-diagnose [bugs][1]. We should remove `makeDefaultExport` in version 5.0. This update

* adds a deprecation warning in preparation for removing `makeDefaultExport`.
* adds a debug build with the deprecation enabled.

Addresses ember-cli#114

[1]:mike-north/ember-lodash#104
lennyburdette pushed a commit to lennyburdette/loader.js that referenced this pull request Jun 23, 2017
…lt` exports

If a module does not define a default export, loader.js assigns `exports.default = exports` for compatibility with modules that don't use `_interopRequireDefault` from babel6. Mutating exports is unexpected behavior and causes difficult-to-diagnose [bugs][1]. We should remove `makeDefaultExport` in version 5.0. This update

* adds a deprecation warning in preparation for removing `makeDefaultExport`.
* adds a debug build with the deprecation enabled.

Addresses ember-cli#114

[1]:mike-north/ember-lodash#104
lennyburdette pushed a commit to lennyburdette/loader.js that referenced this pull request Jun 24, 2017
…lt` exports

If a module does not define a default export, loader.js assigns `exports.default = exports` for compatibility with modules that don't use `_interopRequireDefault` from babel6. Mutating exports is unexpected behavior and causes difficult-to-diagnose [bugs][1]. We should remove `makeDefaultExport` in version 5.0. This update

* adds a deprecation warning in preparation for removing `makeDefaultExport`.
* adds a debug build with the deprecation enabled.

Addresses ember-cli#114

[1]:mike-north/ember-lodash#104
lennyburdette pushed a commit to lennyburdette/loader.js that referenced this pull request Jun 24, 2017
…lt` exports

If a module does not define a default export, loader.js assigns `exports.default = exports` for compatibility with modules that don't use `_interopRequireDefault` from babel6. Mutating exports is unexpected behavior and causes difficult-to-diagnose [bugs][1]. We should remove `makeDefaultExport` in version 5.0. This update

* adds a deprecation warning in preparation for removing `makeDefaultExport`.
* adds a debug build with the deprecation enabled.

Addresses ember-cli#114

[1]:mike-north/ember-lodash#104
lennyburdette pushed a commit to lennyburdette/loader.js that referenced this pull request Aug 1, 2017
…lt` exports

If a module does not define a default export, loader.js assigns `exports.default = exports` for compatibility with modules that don't use `_interopRequireDefault` from babel6. Mutating exports is unexpected behavior and causes difficult-to-diagnose [bugs][1]. We should remove `makeDefaultExport` in version 5.0. This update

* adds a deprecation warning in preparation for removing `makeDefaultExport`.
* adds a debug build with the deprecation enabled.

Addresses ember-cli#114

[1]:mike-north/ember-lodash#104
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