Navigation Menu

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: add Mocha to global variable #4401

Merged

Conversation

irrationnelle
Copy link
Contributor

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions.

Description of the Change

To apply UMD pattern, Mocha module was located in mocha as module name. But browsers are be able to access Mocha as global variable with this change.

I added rollup config footer to assign Mocha module to global variable. I hope that this would be the alias to acutal Mocha module, not deep cloning another Mocha.

Alternate Designs

in browser-entry.js from line 213

mocha.Mocha = Mocha;
mocha.mocha = mocha;

I would like to suggest like next

global.Mocha = Mocha;
global.mocha = mocha;

OR,

mocha.Mocha = Mocha;
mocha.mocha = mocha;

(function(currentGlobal) {
  currentGlobal.Mocha = mocha.Mocha;
})(global);

anyway, I was afraid of any side-effects and then I selected this version as PR.

Benefits

  • Users who used mocha for browsers could update the latest mocha
  • Other third party modules with global variable Mocha could update the latest mocha

Possible Drawbacks

There would be duplicated Mocha modules on browser.
IMHO, an alias for this module would be better than assign object but I would like to avoid to modify core code.

Applicable issues

#4395

Versioning Info

  • Is this a breaking change (major release)? No
  • Is it an enhancement (minor release)? No
  • Is it a bug fix, or does it not impact production code (patch release)? Yes

@jsf-clabot
Copy link

jsf-clabot commented Aug 8, 2020

CLA assistant check
All committers have signed the CLA.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 93.849% when pulling 814338e on irrationnelle:fix/global-variable-for-Mocha into 78d979d on mochajs:master.

@boneskull boneskull added the type: bug a defect, confirmed by a maintainer label Aug 14, 2020
@boneskull boneskull added this to the next milestone Aug 14, 2020
@boneskull boneskull merged commit 2b8a1ff into mochajs:master Aug 14, 2020
@boneskull
Copy link
Member

LGTM, thanks

@boneskull
Copy link
Member

maybe putting this in browser-entry.js would be better, but we can change it later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug a defect, confirmed by a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants