Skip to content

Fix exceptions when running grunt test --debug #4164

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

Merged
merged 1 commit into from
Jun 22, 2015

Conversation

sophiebits
Copy link
Collaborator

No description provided.

@sophiebits
Copy link
Collaborator Author

@jimfb?

@@ -6374,4 +6374,6 @@ CustomElements.addModule(function(scope) {
window.Platform = scope;
})(window.WebComponents);

module.exports = window.WebComponents;
if (typeof exports !== 'undefined') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be typeof module? Also, fixed in #4150

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For whatever reason, this seems to be the standard check for a UMD wrapper:

https://github.com/umdjs/umd/blob/81c95124cbdcb3fce883418ba2882d9cd7d9b2c4/returnExports.js#L21

Copy link
Contributor

Choose a reason for hiding this comment

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

Ugg, maybe we check for both? It seems highly awkward to attempt to write to a variable called 'module' just because a variable called 'exports' exists (especially since the line would crash fatally if module doesn't exist).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the intention is probably to crash if exports exists but module doesn't because the wrapper doesn't know how to deal with that. I'll leave as-is because I think either should work fine and this seems standard.

@sebmarkbage
Copy link
Collaborator

oh

sophiebits added a commit that referenced this pull request Jun 22, 2015
Fix exceptions when running `grunt test --debug`
@sophiebits sophiebits merged commit 7e0a544 into facebook:master Jun 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants