Support ES modules with .js extension and package type module

[JannesMeyer]

I tried to use the new ES module support from version 3.6.2 with some *.js files that are generated via TypeScript.

The 3.6.2 release notes only mention *.mjs files, but I can't see any information on how to use ES modules with "type": "module" in package.json.

Unfortunately, TypeScript cannot emit *.mjs files yet. There is an open issue on the project, but it seems to be far from resolved: microsoft/TypeScript#18442

As an alternative I am using "type": "module", which is supported by node. However, it doesn't seem to be supported by Jasmine yet.

Node version: 15.0.1
Jasmine version: 3.6.2

[JannesMeyer]

If anyone else stumbles on this issue, here is the workaround that I am using in the meantime until official support arrives:

import glob from 'glob';
import Jasmine from 'jasmine';

glob('./dist/spec/**/*.js', async (err, files) => {
  if (err) { throw err; }
  let jasmine = new Jasmine();
  await Promise.all(files.map(f => import(f)));
  jasmine.execute();
});

I save this as jasmine.js and run it with node jasmine.js.

The only problem with it is that I'm losing the ability to pass CLI arguments and IDE integration suffers a bit. Anyway, Jasmine is amazing for making it so easy to run the tests from arbitrary sources!

[sgravrock]

Loading files as ES modules when type is set to module is on my to-do list. In the meantime, I'd be happy to review a pull requests that adds that functionality.

[JannesMeyer]

I would be happy to work on this, even though I am not familiar with the Jasmine codebase yet.

I suppose the first step is to decide which detection logic to use. As far as I am aware, there is no specific Node API to detect the module type of a file.

Option 1:
Load the package.json file that is relevant to the specDir and read the type property from the object, if it's missing default to CommonJS.

I think it would be easiest to use something like path.join(this.projectBaseDir, 'package.json'). However, this could possibly come with some edge cases. I don't know how likely this is going to occur in the real world, but it's possible for a project to have multiple package.json files.

In theory, each sub-directory could have its own package.json and Node seems to respect the settings of the 'closest' package.json.

Option 2:
Call require(...) inside of a try { } catch { } block. After doing some quick testing, I noticed that the error code is always 'ERR_REQUIRE_ESM' when the module is an ES module. In that case we could fall back to an import(...) call. It would look somewhat like this (pseudo code):

let file = './path/to/some-es-module.js';
try {
  require(file);
} catch (e) {
  if (e.code === 'ERR_REQUIRE_ESM') {
    import(file);
  } else {
    throw e;
  }
}

I found another instance where you already rely on an Error's code value, so hopefully it wouldn't be considered too fragile:

jasmine-npm/lib/jasmine.js

Line 113 in 5fc7e6a

if(configFilePath || e.code != 'MODULE_NOT_FOUND') { throw e; }

Not sure if there is any other way to detect the file type that I am not aware of. I'm still looking at the documentation trying to find alternatives.

[JannesMeyer]

After thinking about this for a little bit I would probably favour the second option, because it allows us to have exactly the same detection algorithm as Node without having to worry about re-implementing it (or tracking possible future changes)

[jehon]

Looking into documentation, I wonder if "import" could be used t import both cjs and esm... Reading it here: https://nodejs.org/api/esm.html#esm_interoperability_with_commonjs

This is the test I did:

main.mjs:
import('./test1.cjs').then(data => console.log("cjs", data));
test1.cjs:

module.exports.default = 1;
module.exports.b = 2;

output
cjs [Module] { b: 2, default: { default: 1, b: 2 } }

But this would be a breaking change, since it requires the main.mjs to be an esm module.

[JannesMeyer]

That's a good point. Node.js seems to allow importing CommonJS modules with import().

Does it even require main.mjs to be an ES module? The import() function is available in CommonJS modules as well. The major drawback would be that Node.js 10 doesn't support it out of the box (you still had to enable --experimental-modules back then).

In conclusion, using that strategy would definitely be a breaking change because it would drop support for Node.js 10. You can see Jasmine's officially supported Node versions here:

jasmine-npm/package.json

Line 35 in 5fc7e6a

"node": "^10 || ^12 || ^14"

Btw, not sure if you noticed it, but I had already created pull request #173 which uses require() and then falls back to import() (it's linked right above your comment). This is my first pull request for this project and I am hoping to get some feedback from the maintainers.

[frank-dspeed]

i need to point out !!!! that no detection of the cases is needed

Node Top Level Import allows Import of CJS and ESM

import 'my.cjs'

so you can always go for Top Level Import no dynamic import needed. And also other big news Node Now got Top Level Await also so you could do await import() but as sayed before general top level import for everything is fine already

[JannesMeyer]

That’s correct, but in this case we are talking about a situation where:

1. Maintaining compatibility with Node 10 is desired
2. The filename is in a variable

[sgravrock]

I spent some time both experimenting with Node's actual behavior and digging into the spec a bit. Since ES modules and CommonJS modules have different semantics, I wanted to answer two questions:

1. Will switching from require('./foo') to import('./foo.js') change the behavior of the code in foo.js?
2. How reliable is the answer to the first question? Can we count on it to not change, or might it be an accident of how Node currently implements dynamic import?

For the first question, I used a few different approaches to try to figure out what kind of module the file was being loaded as. For each approach I tried loading the module via require, dynamic import, and static import. In all cases the module’s file extension was .js and I did not have ”type”: “module” set in package.json. The results were the same for Node 10 with --experimental-modules, Node 12, and Node 14:

Loading method	exports supported?	Defaults to strict mode?	module.parent defined?
require	no	no	yes
dynamic import	no	no	no
static import	no	no	no

That's not a complete exploration of all the possible differences. But it's enough to show that loading a CommonJS module via import() can change its behavior. It's possible that there are more important differences than whether or not module.parent is defined.

For the second question, I can't find anything in either the Node documentation or the dynamic import proposal that discusses using either static or dynamic import to load CommonJS modules. It could be that the Node team thought carefully about what should happen when a CommonJS module is imported and made a decision that they plan to stick with, or it could be that the current behavior is an accidental result of how things are implemented. In any case, I don’t see anything in the draft proposal that prevents future versions of Node from either defaulting to ES module semantics or disallowing import() of CommonJS modules altogether. (If anyone has a resource that says otherwise, please post a link. I certainly could have missed something.)

So where does that leave us?

• TypeScript users can’t use ES modules today.
• Having to use .mjs to get ES module semantics is likely to be more and more unpopular as time goes on and more users switch to pure ES module setups.
• We can’t use try/catch to decide how to load a module, because a module might load without throwing but behave differently when loaded the wrong way.
• Defaulting to import() is a breaking change, so we can’t do it in 3.x.
• We’ll still need to support require() for the foreseeable future, both for users who need full CommonJS semantics and in case the behavior of importing a CommonJS module changes.

We could check the package type as described in the title of this issue. That’s slightly fiddly (we’d need to make sure we check the correct package.json in cases where the imported module is in a different package). But it should be pretty robust, and it should just work without users having to do any configuration.

Thoughts?

[frank-dspeed]

@sgravrock even if this is the wrong topic for that but we in nodejs got import { createRequire } from 'module'

and can this way init a require = createRequrire(import.,meta.url);

we are also able to change the behavior of all loaders via hooks and we are able to write a own require shim via dynamic import like

async function require(path) {
	let _module = window.module;
	window.module = {};
	await import(path);
	let exports = module.exports;
	window.module = _module; // restore global
	return exports;
}

(async () => { // top-level await cannot come soon enough…

let x = await require("https://cdn.jsdelivr.net/gh/x/lib/index.js");

})();

hope that helps you a bit while this is true not everything we have but we can not write books in so short time

[jehon]

@sgravrock : here is the doc to the dynamic import in node JS: https://nodejs.org/dist/latest-v14.x/docs/api/esm.html#esm_import_expressions

[niyarlatotep]

@sgravrock sorry for a stupid question but cant' we just simply read package.json from current working dir?

[sgravrock]

@sgravrock sorry for a stupid question but can't we just simply read package.json from current working dir?

Not a stupid question at all. The problem is that the module being imported may be in a different package than the one the current working dir is in. That's actually pretty common. In normal Jasmine usage, the current working dir is in the user's package but all library code, including Jasmine's, is in other packages. We need to make sure that Jasmine's own helpers as well as any other helper files that come from libraries are loaded correctly even when the library and the user's package have different package types.

[sgravrock]

@jehon If I'm reading it correctly, that doc says that import statements can be used to import either ES or CommonJS modules. It doesn't explicitly say whether dynamic import can be used to import CommonJS modules, but I think it's reasonable to infer that it can. I'm not sure how to reconcile that with the differences I was able to observe between the CommonJS environment and the environment provided by both static import and dynamic import.

I want to be able to just import everything and let Node figure it out. But I'm worried about breaking things, especially since I don't have access to a large set of real-world Jasmine test suites to try it on. Maybe one option would be to add an off-by-default option to use dynamic import for everything, and then (if all goes well) make it the default in the next major release.

[frank-dspeed]

@sgravrock import and import() can load both types thats correct but only when the correct package.json is in the module dir at all you should avoid mixing CJS and ESM Code Project Wide so you transpil everything down to CJS or Everything up to ESM

All the Interop stuff is NodeJS Special Magic that only happens in Nodejs and is not Part of any Spec or any Other Environment.

[jehon]

Indeed, import() (and all nodejs stuff) make all the magic when the project is ... correctly configured :-)
Mixing both? I do that, and that's not easy to maintain, but it works.

@sgravrock : I agree with you, I would do that: add a flag in a minor release to allow esm loading with "import", and then make that mandatory for the next major release. A bit more complex to maintain in the meantime, but more "error" prone.

Here, in jasmine, we "import" packages, but we don't care about the "exported" stuff from the package, so this should be the easy path... Stuff become more complex when dealing with exported stuff from cjs into esm (to my experience).

[sgravrock]

Hmm. Jasmine fails to import its own specs on Windows, when configured to use dynamic import for everything:

> node ./bin/jasmine.js

About to import C:/Users/circleci/project/spec/command_spec.js via url: file:///C:/Users/circleci/project/spec/command_spec.js
Error: While loading C:/Users/circleci/project/spec/command_spec.js: Error: Not supported
    at C:\Users\circleci\project\lib\loader.js:25:30
    at async Jasmine._loadFiles (C:\Users\circleci\project\lib\jasmine.js:99:5)
    at async Jasmine.loadSpecs (C:\Users\circleci\project\lib\jasmine.js:90:3)
    at async Jasmine.execute (C:\Users\circleci\project\lib\jasmine.js:282:3)
npm ERR! Test failed.  See above for more details.

Exited with code exit status 1

I don't have easy access to Windows, so it might take a while to get to the bottom of this. If any of you have Windows & would be willing to try to figure out what's wrong on the windows-debugging branch, it would be a big help.

[sgravrock]

Also, it looks like Node 10 import loads .js files as CommonJS modules (albeit without module.parent) even if "type": "module" is set and the --experimental-modules flag is passed.

[sgravrock]

It turns out that the problem isn't Windows. It's that importing .js files in Node 12 is a compatibility minefield, with different point releases behaving very differently:

• Anything before 12.17 can't import .js files at all unless Node is run with —experimental-modules, regardless of what package.json looks like. The error varies from version to version but they all fail.
• There's a range of versions in the middle that can't run a top-level .js file (node somefile.js) if "type": "module" is set. (At least 12.14.0 through 12.16.0.)
• 12.17 is the first version I’ve found that appears to work like 14.x.

Unlike Node 10, we’re going to be living with Node 12 for years to come. So I think we need to be more cautious about this. Using dynamic import for .js files in packages that don’t have ”type": "module” is probably not an option for the foreseeable future. One option would be to both check the nearest package.json and make the feature opt-in until 4.0, at which point it would be opt-out.

[sgravrock]

Ok, I think I have something that works reliably in Node <= 12.17 and fails gracefully in older versions. I'm making it opt-in (by setting "jsLoader": "import" in jasmine.json) for now but hopefully that can become the default in the future. @JannesMeyer @jehon if you get a chance to try this out before it's released, I'd appreciate feedbaack.

[roger-marley-lrn]

@sgravrock hi, testing this new version but still having problems with this, when I enable that jsLoader setting (with type:module in package.json also), it still can't follow import statements unless I include .js at the end of them (which obviously I don't want to do). I.e. import A from 'name' won't work but import A from 'name.js' will. Anything else I need to do?

[sgravrock]

@roger-marley-lrn You probably have to use the .js extension. That's (mostly) how ES module import specifiers work in Node.. The extension is required in most situations with the exception of importing a bare package name (import {something} from 'jasmine') or importing a subpath from a package that uses the "exports" field. As far as I can tell that means that you'll have to use the extension in most if not all situations where you're importing your own code from your specs. This came as a surprise to a lot of people, including me, especially given the prevalence of compiles-to-JS languages that use syntax similar to ES modules and don't require the extension.

If you're curious, there's some background information in these threads:
nodejs/modules#444
nodejs/modules#268

[roger-marley-lrn]

@sgravrock I see, well currently I have to transpile ESM back into CJS in order to execute a old jasmine test suite (that I'm busy resurrecting) against it due to this, since the whole codebase its to be executed against uses .js extension files containing ESM using bare imports, and I wouldn't be able to get the suggestion to rewrite hundreds of import statements that work under normal operation in order to have jasmine tests will work past PR. I'll do some reading and look into those links you've suggested (cheers!) and see if there's some switch I can pass to node or something to have it tolerate the bare imports.

[frank-dspeed]

@roger-marley-lrn as node resolve works inside ESM you can import anything that has a package.json via bare identifier

[roger-marley-lrn]

Alright looks like executing with NODE_OPTIONS=--es-module-specifier-resolution=node + having "type": "module" in package.json + using a config jasmine.json containing "jsLoader": "import" is sufficient to have jasmine tests execute direct against ESM .js files using bare imports, cheers @sgravrock including for the recent releases

[fuweichin]

To get out of trouble from the scenario (esm source + .js extension) , you'd better

• install mainline version of Node.js
• install latest Jasmine
• follow jasmine official guide using-es-modules

Or if you use older version of Node.js / Jasmine and cannot upgrade them easily, try to install and use 'jasmine-esm' instead of 'jasmine', replace terminal command jasmine with jasmineEsm.

Example Scenario:

nodejs version: 16.10
jasmine version: 3.9.0
file extension: .js
source type: esm

spec/a.spec.js

import path from 'path';

Test Result:

jasmine.json \ package.json	without "type":"module"	with"type":"module"
without "jsLoader": "import"; run jasmine	✕1	✕2
with "jsLoader": "import"; run jasmine	✕1	✓
without "jsLoader": "import"; run jasmineEsm3	✕1	✓

Footnotes

1. SyntaxError: Cannot use import statement outside a module ↩ ↩² ↩³

2. Error [ERR_REQUIRE_ESM]: require() of ES Module ... from ...\jasmine\lib\loader.js not supported. ↩

3. can be installed with npm install -g jasmine-esm, run jasmineEsm just like the way you run jasmine ↩