Opt-in support for ES modules with .js extension

* Uses import rather than require if jsLoader: "import" is set in config
* Opt-in for now. import is supposed to work for all CommonJS modules too, but
  there are subtle differences.
* Fixes #170.

.eslintignore

@@ -1 +1,2 @@
 spec/fixtures/cjs-syntax-error/syntax_error.js
+spec/fixtures/js-loader-import/*.js

README.md

@@ -61,6 +61,20 @@ jasmine JASMINE_CONFIG_PATH=relative/path/to/your/jasmine.json
 jasmine --config=relative/path/to/your/jasmine.json
 ```
 
+## Using ES modules
+
+If the name of a spec file or helper file ends in `.mjs`, Jasmine will load it
+as an [ES module](https://nodejs.org/docs/latest-v13.x/api/esm.html) rather 
+than a CommonJS module. This allows the spec file or helper to import other 
+ES modules. No extra configuration is required.
+
+You can also use ES modules with names ending in `.js` by adding 
+`"jsLoader": "import"` to `jasmine.json`. This should work for CommonJS modules
+as well as ES modules. We expect to make it the default in a future release.
+Please [log an issue](https://github.com/jasmine/jasmine-npm/issues) if you have
+code that doesn't load correctly with `"jsLoader": "import"`.
+
+
 # Filtering specs
 
 Execute only those specs which filename match given glob:

lib/jasmine.js

@@ -87,18 +87,23 @@ Jasmine.prototype.addMatchers = function(matchers) {
 };
 
 Jasmine.prototype.loadSpecs = async function() {
-  for (const file of this.specFiles) {
-    await this.loader.load(file);
-  }
+  await this._loadFiles(this.specFiles);
 };
 
 Jasmine.prototype.loadHelpers = async function() {
-  for (const file of this.helperFiles) {
-    await this.loader.load(file);
+  await this._loadFiles(this.helperFiles);
+};
+
+Jasmine.prototype._loadFiles = async function(files) {
+  for (const file of files) {
+    await this.loader.load(file, this._alwaysImport || false);
   }
+
 };
 
 Jasmine.prototype.loadRequires = function() {
+  // TODO: In 4.0, switch to calling _loadFiles
+  // (requires making this function async)
   this.requires.forEach(function(r) {
     require(r);
   });
@@ -135,6 +140,16 @@ Jasmine.prototype.loadConfig = function(config) {
     configuration.random = config.random;
   }
 
+  if (config.jsLoader === 'import') {
+    this._alwaysImport = true;
+  } else if (config.jsLoader === 'require' || config.jsLoader === undefined) {
+    this._alwaysImport = false;
+  } else {
+    throw new Error(`"${config.jsLoader}" is not a valid value for the ` +
+      'jsLoader configuration property. Valid values are "import", ' +
+      '"require", and undefined.');
+  }
+
   if (Object.keys(configuration).length > 0) {
     this.env.configure(configuration);
   }

lib/loader.js

@@ -6,8 +6,8 @@ function Loader(options) {
   this.import_ = options.importShim || importShim;
 }
 
-Loader.prototype.load = function(path) {
-  if (path.endsWith('.mjs')) {
+Loader.prototype.load = function(path, alwaysImport) {
+  if (alwaysImport || path.endsWith('.mjs')) {
     // The ES module spec requires import paths to be valid URLs. As of v14,
     // Node enforces this on Windows but not on other OSes.
     const url = `file://${path}`;

spec/fixtures/js-loader-default/aSpec.js

@@ -0,0 +1,5 @@
+describe('a file with js extension', function() {
+  it('was loaded as a CommonJS module', function() {
+    expect(module.parent).toBeTruthy();
+  });
+});

spec/fixtures/js-loader-default/jasmine.json

@@ -0,0 +1,4 @@
+{
+  "spec_dir": ".",
+  "spec_files": ["aSpec.js"]
+}

spec/fixtures/js-loader-import/anEsModule.js

@@ -0,0 +1,3 @@
+export function foo() {
+  return 42;
+}

spec/fixtures/js-loader-import/anEsModuleSpec.js

@@ -0,0 +1,7 @@
+import {foo} from './anEsModule.js';
+
+describe('foo', function() {
+  it('returns 42', function() {
+    expect(foo()).toEqual(42);
+  });
+});

spec/fixtures/js-loader-import/jasmine.json

@@ -0,0 +1,5 @@
+{
+  "spec_dir": ".",
+  "spec_files": ["anEsModuleSpec.js"],
+  "jsLoader": "import"
+}

spec/fixtures/js-loader-import/package.json

@@ -0,0 +1,3 @@
+{
+  "type": "module"
+}

spec/fixtures/js-loader-require/aSpec.js

@@ -0,0 +1,5 @@
+describe('a file with js extension', function() {
+  it('was loaded as a CommonJS module', function() {
+    expect(module.parent).toBeTruthy();
+  });
+});

spec/fixtures/js-loader-require/jasmine.json

@@ -0,0 +1,5 @@
+{
+  "spec_dir": ".",
+  "spec_files": ["aSpec.js"],
+  "jsLoader": "require"
+}

spec/integration_spec.js

@@ -1,6 +1,27 @@
 const child_process = require('child_process');
 
 describe('Integration', function () {
+  beforeEach(function() {
+    jasmine.addMatchers({
+      toBeSuccess: function(matchersUtil) {
+        return {
+          compare: function(actual, expected) {
+            const result = { pass: actual.exitCode === 0 };
+
+            if (result.pass) {
+              result.message = 'Expected process not to succeed but it did.';
+            } else {
+              result.message = `Expected process to succeed but it exited ${actual.exitCode}.`;
+            }
+
+            result.message += '\n\nOutput:\n' + actual.output;
+            return result;
+          }
+        };
+      }
+    });
+  });
+
   it('supports ES modules', async function () {
     let {exitCode, output} = await runJasmine('spec/fixtures/esm');
     expect(exitCode).toEqual(0);
@@ -20,6 +41,18 @@ describe('Integration', function () {
     );
   });
 
+  it('loads .js files using import when jsLoader is "import"', async function() {
+    expect(await runJasmine('spec/fixtures/js-loader-import')).toBeSuccess();
+  });
+
+  it('loads .js files using require when jsLoader is "require"', async function() {
+    expect(await runJasmine('spec/fixtures/js-loader-require')).toBeSuccess();
+  });
+
+  it('loads .js files using require when jsLoader is undefined', async function() {
+    expect(await runJasmine('spec/fixtures/js-loader-default')).toBeSuccess();
+  });
+
   it('handles load-time exceptions from CommonJS specs properly', async function () {
     const {exitCode, output} = await runJasmine('spec/fixtures/cjs-load-exception');
     expect(exitCode).toEqual(1);

spec/jasmine_spec.js

@@ -175,8 +175,10 @@ describe('Jasmine', function() {
 
   describe('loading configurations', function() {
     beforeEach(function() {
+      this.loader = jasmine.createSpyObj('loader', ['load']);
       this.fixtureJasmine = new Jasmine({
         jasmineCore: this.fakeJasmineCore,
+        loader: this.loader,
         projectBaseDir: 'spec/fixtures/sample_project'
       });
     });
@@ -273,6 +275,47 @@ describe('Jasmine', function() {
         });
       });
 
+      describe('with jsLoader: "require"', function () {
+        it('tells the loader not to always import', async function() {
+          this.configObject.jsLoader = 'require';
+
+          this.fixtureJasmine.loadConfig(this.configObject);
+          await this.fixtureJasmine.loadSpecs();
+
+          expect(this.loader.load).toHaveBeenCalledWith(jasmine.any(String), false);
+        });
+      });
+
+      describe('with jsLoader: "import"', function () {
+        it('tells the loader to always import', async function() {
+          this.configObject.jsLoader = 'import';
+
+          this.fixtureJasmine.loadConfig(this.configObject);
+          await this.fixtureJasmine.loadSpecs();
+
+          expect(this.loader.load).toHaveBeenCalledWith(jasmine.any(String), true);
+        });
+      });
+
+      describe('with jsLoader set to an invalid value', function () {
+        it('throws an error', function() {
+          this.configObject.jsLoader = 'bogus';
+          expect(() => {
+            this.fixtureJasmine.loadConfig(this.configObject);
+          }).toThrowError(/"bogus" is not a valid value/);
+        });
+      });
+
+      describe('with jsLoader undefined', function () {
+        it('tells the loader not to always import', async function() {
+          this.configObject.jsLoader = undefined;
+
+          this.fixtureJasmine.loadConfig(this.configObject);
+          await this.fixtureJasmine.loadSpecs();
+
+          expect(this.loader.load).toHaveBeenCalledWith(jasmine.any(String), false);
+        });
+      });
     });
 
     describe('from a file', function() {

spec/loader_spec.js

@@ -6,72 +6,88 @@ describe('loader', function() {
   });
 
   describe('#load', function() {
-    describe('When the path ends in .mjs', function () {
-      it('loads the file as an es module', async function () {
-        const requireShim = jasmine.createSpy('requireShim');
-        let resolve;
-        const importPromise = new Promise(function (res) {
-          resolve = res;
-        });
-        const importShim = jasmine.createSpy('importShim')
-          .and.returnValue(importPromise);
-        const loader = new Loader({requireShim, importShim});
-
-        const loaderPromise = loader.load('./foo/bar/baz.mjs');
-
-        expect(requireShim).not.toHaveBeenCalled();
-        expect(importShim).toHaveBeenCalledWith('file://./foo/bar/baz.mjs');
-        await expectAsync(loaderPromise).toBePending();
+    describe('With alwaysImport: true', function() {
+      describe('When the path ends in .mjs', function () {
+        esModuleSharedExamples('mjs', true);
+      });
 
-        resolve();
+      describe('When the path does not end in .mjs', function () {
+        esModuleSharedExamples('js', true);
+      });
+    });
 
-        await expectAsync(loaderPromise).toBeResolved();
+    describe('With alwaysImport: false', function() {
+      describe('When the path ends in .mjs', function () {
+        esModuleSharedExamples('mjs', false);
       });
 
-      it("adds the filename to errors that don't include it", async function() {
-        const underlyingError = new SyntaxError('some details but no filename, not even in the stack trace');
-        const importShim = () => Promise.reject(underlyingError);
-        const loader = new Loader({importShim});
+      describe('When the path does not end in .mjs', function () {
+        it('loads the file as a commonjs module', async function () {
+          const requireShim = jasmine.createSpy('requireShim')
+            .and.returnValue(Promise.resolve());
+          const importShim = jasmine.createSpy('importShim');
+          const loader = new Loader({requireShim, importShim});
 
-        await expectAsync(loader.load('foo.mjs')).toBeRejectedWithError(
-          "While loading foo.mjs: SyntaxError: some details but no filename, not even in the stack trace"
-        );
-      });
+          await expectAsync(loader.load('./foo/bar/baz', false)).toBeResolved();
+
+          expect(requireShim).toHaveBeenCalledWith('./foo/bar/baz');
+          expect(importShim).not.toHaveBeenCalled();
+        });
 
-      it('propagates errors that already contain the filename without modifying them', async function () {
-        const requireShim = jasmine.createSpy('requireShim');
-        const underlyingError = new Error('nope');
-        underlyingError.stack = underlyingError.stack.replace('loader_spec.js', 'foo.mjs');
-        const importShim = jasmine.createSpy('importShim')
-          .and.callFake(() => Promise.reject(underlyingError));
-        const loader = new Loader({requireShim, importShim});
+        it('propagates the error when import fails', async function () {
+          const underlyingError = new Error('nope');
+          const requireShim = jasmine.createSpy('requireShim')
+            .and.throwError(underlyingError);
+          const importShim = jasmine.createSpy('importShim');
+          const loader = new Loader({requireShim, importShim}, false);
 
-        await expectAsync(loader.load('foo.mjs')).toBeRejectedWith(underlyingError);
+          await expectAsync(loader.load('foo')).toBeRejectedWith(underlyingError);
+        });
       });
     });
+  });
+});
 
-    describe('When the path does not end in .mjs', function () {
-      it('loads the file as a commonjs module', async function () {
-        const requireShim = jasmine.createSpy('requireShim')
-          .and.returnValue(Promise.resolve());
-        const importShim = jasmine.createSpy('importShim');
-        const loader = new Loader({requireShim, importShim});
+function esModuleSharedExamples(extension, alwaysImport) {
+  it('loads the file as an es module', async function () {
+    const requireShim = jasmine.createSpy('requireShim');
+    let resolve;
+    const importPromise = new Promise(function (res) {
+      resolve = res;
+    });
+    const importShim = jasmine.createSpy('importShim')
+      .and.returnValue(importPromise);
+    const loader = new Loader({requireShim, importShim});
 
-        await expectAsync(loader.load('./foo/bar/baz')).toBeResolved();
+    const loaderPromise = loader.load(`./foo/bar/baz.${extension}`, alwaysImport);
 
-        expect(requireShim).toHaveBeenCalledWith('./foo/bar/baz');
-        expect(importShim).not.toHaveBeenCalled();
-      });
+    expect(requireShim).not.toHaveBeenCalled();
+    expect(importShim).toHaveBeenCalledWith(`file://./foo/bar/baz.${extension}`);
+    await expectAsync(loaderPromise).toBePending();
 
-      it('propagates the error when import fails', async function () {
-        const underlyingError = new Error('nope');
-        const requireShim = jasmine.createSpy('requireShim')
-          .and.throwError(underlyingError);
-        const importShim = jasmine.createSpy('importShim');
-        const loader = new Loader({requireShim, importShim});
+    resolve();
 
-        await expectAsync(loader.load('foo')).toBeRejectedWith(underlyingError);
-      });
-    });
+    await expectAsync(loaderPromise).toBeResolved();
   });
-});
+
+  it("adds the filename to errors that don't include it", async function() {
+    const underlyingError = new SyntaxError('some details but no filename, not even in the stack trace');
+    const importShim = () => Promise.reject(underlyingError);
+    const loader = new Loader({importShim});
+
+    await expectAsync(loader.load(`foo.${extension}`, alwaysImport)).toBeRejectedWithError(
+      `While loading foo.${extension}: SyntaxError: some details but no filename, not even in the stack trace`
+    );
+  });
+
+  it('propagates errors that already contain the filename without modifying them', async function () {
+    const requireShim = jasmine.createSpy('requireShim');
+    const underlyingError = new Error('nope');
+    underlyingError.stack = underlyingError.stack.replace('loader_spec.js', `foo.${extension}`);
+    const importShim = jasmine.createSpy('importShim')
+      .and.callFake(() => Promise.reject(underlyingError));
+    const loader = new Loader({requireShim, importShim});
+
+    await expectAsync(loader.load(`foo.${extension}`, alwaysImport)).toBeRejectedWith(underlyingError);
+  });
+}