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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better ESM support #1728

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

NullVoxPopuli
Copy link

@NullVoxPopuli NullVoxPopuli commented Oct 14, 2023

Resolves: #1724

(however, I can't run tests locally, because the browser launcher is puppeteer, and some ancient browser that I don't have installed 馃槄

>> There was an error with headless chrome
Fatal error: Could not find browser revision 818858. Run "PUPPETEER_PRODUCT=firefox npm install" or "PUPPETEER_PRODUCT=firefox yarn install" to download a supported Firefox browser binary.

)

I had started with an esm.html file to test with, but script.type=module doesn't allow the file protocol when loading modules, so we'd have to use an http server, but then the relative file paths for the link / importmap / script.src wouldn't work the same.

@NullVoxPopuli NullVoxPopuli changed the title Create node test Better ESM support Oct 14, 2023
@@ -21,6 +21,11 @@ export default function exportQUnit (QUnit) {

Copy link
Member

Choose a reason for hiding this comment

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

Do you know what it is about module.exports = QUnit; that doesn't work for ESM compat layers? It seems odd to assign module.exports = QUnit; but then (seemingly redundantly) also assign module.exports.test = QUnit.test; to itself. If the transformation layers are based on static analysis, that would explain it, but I thought they were runtime based?

Copy link
Author

Choose a reason for hiding this comment

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

maybe. as I dug in to this, I think it'd be better to have actual separate builds -- and we're already in a good spot since it seems QUnit is written in ESM -- so we'd just emit a new bundle and wire up package.json#exports so that the import/require/etc are routed correctly.

related questions:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Qunit v2 has incorrectly configured exports
2 participants