-
Notifications
You must be signed in to change notification settings - Fork 778
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
Better ESM support #1728
Better ESM support #1728
Conversation
@@ -21,6 +21,11 @@ export default function exportQUnit (QUnit) { | |||
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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:
@NullVoxPopuli Can you confirm that the QUnit 3.0.0-alpha.4 release solves this? E.g. https://code.jquery.com/qunit/qunit-3.0.0-alpha.4.module.js or https://unpkg.com/[email protected]/qunit/esm/qunit.module.js |
It does! |
You can At least in the few ways we test it at https://github.com/qunitjs/qunit/blob/3.0.0-alpha.4/demos/bundlers/test/import-named.js, but if you find cases where it doesn't work, that'd be good to know! |
You're too fast! I used the wrong punctuation |
Ah... 😅 Thanks for checking! |
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 😅
)
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.