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

Let CLI load ESM files that have a .js suffix #1426

Open
frank-dspeed opened this issue Jan 19, 2020 · 22 comments
Open

Let CLI load ESM files that have a .js suffix #1426

frank-dspeed opened this issue Jan 19, 2020 · 22 comments
Labels
Component: CLI Type: Enhancement New idea or feature request.
Milestone

Comments

@frank-dspeed
Copy link

Tell us about your runtime:

  • QUnit version:
  • What environment are you running QUnit in? (e.g., browser, Node):
  • How are you running QUnit? (e.g., script, testem, Grunt):

What are you trying to do?

i am trying to run a test that is writen in as .mjs file so all this cli stuff does not work as it depends on require.
Code that reproduces the problem:

If you have any relevant configuration information, please include that here:

What did you expect to happen?

implament a esm version

What actually happened?

@trentmwillis
Copy link
Member

@frank-dspeed you should be able to use qunit --require esm with the esm package.

@frank-dspeed
Copy link
Author

@trentmwillis good idea i did it with a little other trick i keep the unit test files as .js and do dynamic import as that allows esm but it would be great if there would be a nativ qunit cli that accepts .mjs input

at present i needed to transform all tests to async because of the dynamic import. ESM Works but it uses a trick and transpils i don't want to go that route i am advocat of standards and transpiling is something that i want to drop all over in all JS Packages. the Time of Transpil Compile build is over. We have terminated all cross environment issues over the last past years now only package maintainers need to follow up.

@trentmwillis
Copy link
Member

That's reasonable; I'd like to see official support in the QUnit CLI for ECMAScript Modules, but the API is currently still "experimental" in Node. Given the limited bandwidth of the maintainers, we'll likely wait until the API reaches "stable" before implementing it.

Until then, our recommendation would be to use esm or some similar tool.

@trentmwillis trentmwillis added Component: CLI Type: Enhancement New idea or feature request. labels Jan 27, 2020
@frank-dspeed
Copy link
Author

@trentmwillis your wrong the new node 12 and 13 have experimental-modules unflagged its time to do it.

Interoperability is finished.

import() works in both cjs and esm to require both

in ESM you can do createRequire to use require and you can as pointed out import()

@frank-dspeed
Copy link
Author

lets create a qunit-es binary that does use import() then all tests can work out of the box no matter if cjs or esm

@trentmwillis
Copy link
Member

Can you please point me to something official from the Node.js organization saying that the API is stable? You've not provided any proof that it actually is, so just saying "your wrong" does not make me inclined to agree with you. On the other hand, I have linked to the official Node.js documentation above stating the API is still "experimental". In fact, you even just referred to it as "experimental-modules".

Note, I'd be open to a PR to implement support for this (with proper tests) for the current API, but as noted above I don't think any of the maintainers currently have time to implement this. And just saying "its time to do it", does not mean that we have to do it. The beauty of open source software is that if you have a solution, you can submit it as a pull request.

@frank-dspeed
Copy link
Author

@trentmwillis simply ignore it or take it i don't care out of my view its total ok if you don't want to jump on that train now. There are alternatives like jest that do that and they have nice importers for qunit tests .

i my self created a import based qunit for my own needs.

You can Open up the issue in 3 years and you will see no change in that api as there are only 2 people working on it import behavior will stay as is modules in general are experimental in nodeJS not the import api.

@sylvainpolletvillard
Copy link

The qunit --require esm solution no longer works since Node 12.16 due to this error: standard-things/esm#868

It it correct to claim that QUnit can not be used to test an ES Module ?

@frank-dspeed
Copy link
Author

frank-dspeed commented Mar 1, 2020

@sylvainpolletvillard that claim is not correct as there is ESM Interoperability via dynamic import

src/my-esm.mjs

export const numberFour = 4

tests/CJS.js

const { test } = QUnit;

test( "an async test", async t => {
  const { numberFour } = await import('../src/my-esm.mjs')
  t.equal( numberFour, 4 );
});

https://api.qunitjs.com/QUnit/test at the end

keep in mind that the rules for ESM interoperability do apply here

Solutions aka the Rules of current ECMAScript Development 2020

  • your source should be in Valid ECMAScript with .mjs extension
  • static type checking via typescript checkJs true and JSDOC annotations in your code.
  • package.json
    • Only devDependencies
    • { "type": "module" } //is the future use commonjs for backward compat
  • vscode don't support .mjs right because of typescript it needs .mjs support workaround is use package.json with type module and then .js at present till .mjs support lands in typescript
  • don't do import without extension don't use package JSON or any lookups in your own code
  • use rollup to build ECMAScript bundles that are consumed
  • dev should not get packaged and stay in git
  • bundles should work without package.json
  • if you want to use node_modules as vendor maintained packages be aware of the fact that they are not clean coded go for es-modules
    • modules can be fetched how you like it via wget or any method and then get bundled up.
  • run inside a container engine to apply isolation and do awsome repeatable infrastructure that scales.

the text is from my Article about ECMAScript 2020

https://gist.github.com/frank-dspeed/de322e0bd7e0daf2dbdb2ccaeee34665

@sylvainpolletvillard
Copy link

The dynamic import solution worked for me. Top level await is not supported, you have to wrap it in a QUnit.test. To avoid reloading the same module in every test, I assigned it to a global variable. I also had to change all my test specs extensions to ".cjs"

qunit test/index.cjs

index.cjs:

QUnit.test("loading module", async t => {
  const globals = await import("../src/mymodule.mjs")
  Object.assign(globalThis, globals)
  t.ok("MyModule" in globals, "libarary correcty loaded as module");
});

require("./test.spec.cjs")
require("./othertest.spec.cjs")
require("./yetanothertest.spec.cjs")

So it works, but the setup is a bit awkward and poorly documented. I hope we will get support for static imports soon with QUnit CLI.

@frank-dspeed
Copy link
Author

@sylvainpolletvillard then you don't readed my Rules correctly :) i need to rewrite them maybe the most easy solution is to store a package.json inside the tests/ folder and content is { type: "commonjs"}
then you can name them .js and inside your main package json you do type: module

node-resolve will always get package.json relativ to the import :)

@frank-dspeed
Copy link
Author

also note that you can go for the npm esm package and use that as loader.

@sylvainpolletvillard
Copy link

you don't read me neither. the esm solution as loader no longer works

@frank-dspeed
Copy link
Author

@sylvainpolletvillard but that it don't works is related to your package,json files. you need to split your es and cjs code into 2 diffrent directorys each with the right package,json

modules without type=module
modules with type=modul

@sylvainpolletvillard
Copy link

I can't split esm and cjs code completely. I would still need to import my sources as ESM at some point in the tests folder, just like you put a dynamic import in your tests/CJS.js file above. The whole point of the esm package was to be able to load my lib as a module in the tests files. It was working fine until Node 12.16.

@frank-dspeed
Copy link
Author

@sylvainpolletvillard is your repo public if yes I can help you a bit and maybe refactor and point out what is wrong there must be something wrong. Anything that I could not see out of this text I work for many years with ESModules I do not get such errors even not when mixed with CJS

@thisismydesign
Copy link

I couldn't get --require esm working. The issue is described perfectly here: https://github.com/jeremiahlee/esm-qunit-experiment

Any up-to-date solutions?

@Krinkle
Copy link
Member

Krinkle commented Jul 20, 2021

@thisismydesign QUnit 2.13.0 and later support ESM out of the box for Node.js 12+ (ref #1510). Does that work? Do the same files work when handled by Node.js directly without QUnit? If not, there may be a differnet issue. An up to date example of what you're trying that isn't working would help pinpoint the issue!

@thisismydesign
Copy link

@Krinkle Thanks for the quick reply! I think the issue might be because our files are still ending with .js. Can I enable ESM support for all files?

Using those versions this is what I see:
qunit tests/unit/adapters**/*.js

not ok 1 tests/unit/adapters/adapter-test.js > Failed to load the test file with error:
/app/tests/unit/adapters/adapter-test.js:1
import { module, test } from 'qunit';
^^^^^^

SyntaxError: Cannot use import statement outside a module

With esm:

qunit tests/unit/adapters**/*.js --require esm

not ok 1 tests/unit/adapters/adapter-test.js > Failed to load the test file with error:
/app/tests/unit/adapters/adapter-test.js:1
import { module, test } from 'qunit';

SyntaxError: The requested module 'file:///app/node_modules/ember-qunit/index.js' does not provide an export named 'setupTest'

(ember-qunit does provide that import, this is also an issue related to modules)

@Krinkle Krinkle added the Type: Question Ask questions or seek assistance. label Jul 25, 2021
@Krinkle Krinkle reopened this Jul 25, 2021
@frank-dspeed
Copy link
Author

@sylvainpolletvillard maybe i should point out that the esm implementation has already a global module cache based on the import url if you point to the same file you will get it directly from the cache.

you can verify that with some side effect if you create a .mjs file that for example does console.log() it will log only 1x even if you import it 100 times. as the esm implementation is url based you can force to get a new module via a hash bang like

import('./me.js#v2')

anyway the point is import returns a pointer to the existing instanciated module that is importent to know.

@Krinkle Krinkle changed the title Let cli work with esm modules Let CLI load ESM files that have a .js suffix Sep 16, 2023
@Krinkle Krinkle removed the Type: Question Ask questions or seek assistance. label Sep 16, 2023
@Krinkle
Copy link
Member

Krinkle commented Jun 18, 2024

I propose we take the same approach as Eleventy in QUnit 3.0 (details at 11ty/eleventy#836 and 11ty/eleventy#3074).

In other words:

  • If local directory package.json contains type:module, load .js as ESM with import. Otherwise, use require.
  • Load .cjs with require.
  • Load .mjs as ESM with import.

Ref https://github.com/11ty/eleventy/blob/v3.0.0-alpha.13/src/Eleventy.js#L978.
Ref https://github.com/11ty/eleventy/blob/v3.0.0-alpha.13/src/Util/JavaScriptDependencies.js#L10-L28.

Note that upstream Node.js has also added support for loading ESM files to require() in nodejs/node#51977, released in Node.js 22 (currently behind experimental flag).

@Krinkle Krinkle added this to the 3.0 release milestone Jun 18, 2024
@frank-dspeed
Copy link
Author

All this is not needed anymore we added flags to the current node versions that even support mixed module loading.

i got no time to go more deep into it but --experimental-require-module

and flags for module detection i can not list them all but some one needs to evaluate that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: CLI Type: Enhancement New idea or feature request.
Development

No branches or pull requests

5 participants