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

Browser: improve 'bdd' import for bundlers #4769

Merged
merged 1 commit into from Oct 15, 2021
Merged

Browser: improve 'bdd' import for bundlers #4769

merged 1 commit into from Oct 15, 2021

Conversation

juergba
Copy link
Member

@juergba juergba commented Oct 14, 2021

Description

History: #4746

Above PR proved to be too restrictive as it's no longer possible to do ES6 style import { describe, it } from 'mocha' while using bundlers.

Description of the Change

browser-entry.js: push all bdd functions from global to mocha.

We have conflicts between global.setup (tdd version of beforeEach hook) and mocha.setup(browser setup), and therefore we don't copy the tdd functions to mocha. The tdd interface has never/won't be available for ES6 style import.

At the time of copying the bdd functions are undefined, nevertheless bundlers seem to rely on those lines. The exact logic remained unclear to me.

Applicable issues

closes #4763

@juergba juergba self-assigned this Oct 14, 2021
@juergba juergba added browser confirmed-bug integration semver-patch labels Oct 14, 2021
@juergba
Copy link
Member Author

@juergba juergba commented Oct 14, 2021

cc @apellerano-pw @PaperStrike

@juergba juergba requested a review from Oct 14, 2021
@apellerano-pw
Copy link

@apellerano-pw apellerano-pw commented Oct 14, 2021

Thank you, @juergba and @PaperStrike ! 🎉

@PaperStrike
Copy link
Contributor

@PaperStrike PaperStrike commented Oct 14, 2021

Actually I'm still confused how it works...

As in #4746, I'm using playwright-test (which uses esbuild internally) to run mocha tests and the only way to achieve import { describe, it } from 'mocha' is the one copy from Mocha.

Since import { describe, it } from 'mocha' never works in my case and the algorithm (to understand and to support all bundlers) seems quite complex, merging this seems to be the best choice. 👍
(Personally I'm looking forward to the ESM mocha. 👀)

@juergba
Copy link
Member Author

@juergba juergba commented Oct 14, 2021

Personally I'm looking forward to the ESM mocha.

Yes, but I'm unsure about the implications, yet.
Will we need globals anymore? I guess no, because a module has its own scope, but Mocha uses globals ever since.
There still remains the conflict on setup(), so evtl. we will need import { describe, it } from 'mocha.bdd'.

@juergba juergba merged commit 012d79d into master Oct 15, 2021
22 checks passed
@juergba juergba deleted the juergba/bundler branch Oct 15, 2021
@juergba juergba added this to the next milestone Oct 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser confirmed-bug integration semver-patch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants