Skip to content
This repository was archived by the owner on Feb 26, 2024. It is now read-only.

fix(mocha): use global if window does not exist#943

Merged
mhevery merged 1 commit intoangular:masterfrom
justindujardin:mocha-nativescript
Dec 27, 2017
Merged

fix(mocha): use global if window does not exist#943
mhevery merged 1 commit intoangular:masterfrom
justindujardin:mocha-nativescript

Conversation

@justindujardin
Copy link
Copy Markdown
Contributor

  • window does not exist in NativeScript applications.
  • this removes a hacky workaround where you assign global.window to global before including the mocha patch.
  • preserve existing behavior by using window first if it exists.

Comment thread lib/mocha/mocha.ts Outdated
})(Mocha.Runner.prototype.runTest, Mocha.Runner.prototype.run);

})(window); No newline at end of file
})(typeof window !== 'undefined' ? window : global);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

please use

})(typeof window !== 'undefined' && window || typeof self !== 'undefined' && self || global);

which is used here https://github.com/angular/zone.js/blob/master/lib/zone.ts#L1344

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

- `window` does not exist in NativeScript applications.
- this removes a hacky workaround where you assign `global.window` to `global` before including the mocha patch.
- preserve existing behavior by using window first if it exists.
@justindujardin
Copy link
Copy Markdown
Contributor Author

The CI build appears to be broken unrelated to this change. System.js is failing to import some rxjs operators.

@JiaLiPassion
Copy link
Copy Markdown
Collaborator

@justindujardin , yes, current CI is broken, you can just ignore that.

@mhevery mhevery merged commit 3caf6fc into angular:master Dec 27, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants