Revert "Opt-in support for ES modules with .js extension"

Does not work on most Node 12.x versions.

This reverts commit 6e6111e.

.eslintignore

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

README.md

@@ -61,20 +61,6 @@ 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,23 +87,18 @@ Jasmine.prototype.addMatchers = function(matchers) {
 };
 
 Jasmine.prototype.loadSpecs = async function() {
-  await this._loadFiles(this.specFiles);
+  for (const file of this.specFiles) {
+    await this.loader.load(file);
+  }
 };
 
 Jasmine.prototype.loadHelpers = async function() {
-  await this._loadFiles(this.helperFiles);
-};
-
-Jasmine.prototype._loadFiles = async function(files) {
-  for (const file of files) {
-    await this.loader.load(file, this._alwaysImport || false);
+  for (const file of this.helperFiles) {
+    await this.loader.load(file);
   }
-
 };
 
 Jasmine.prototype.loadRequires = function() {
-  // TODO: In 4.0, switch to calling _loadFiles
-  // (requires making this function async)
   this.requires.forEach(function(r) {
     require(r);
   });
@@ -140,16 +135,6 @@ 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, alwaysImport) {
-  if (alwaysImport || path.endsWith('.mjs')) {
+Loader.prototype.load = function(path) {
+  if (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/integration_spec.js

@@ -1,27 +1,6 @@
 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);
@@ -41,18 +20,6 @@ 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,10 +175,8 @@ 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'
       });
     });
@@ -275,47 +273,6 @@ 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,88 +6,72 @@ describe('loader', function() {
   });
 
   describe('#load', function() {
-    describe('With alwaysImport: true', function() {
-      describe('When the path ends in .mjs', function () {
-        esModuleSharedExamples('mjs', true);
-      });
-
-      describe('When the path does not end in .mjs', function () {
-        esModuleSharedExamples('js', true);
-      });
-    });
-
-    describe('With alwaysImport: false', function() {
-      describe('When the path ends in .mjs', function () {
-        esModuleSharedExamples('mjs', false);
-      });
-
-      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});
+    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});
 
-          await expectAsync(loader.load('./foo/bar/baz', false)).toBeResolved();
+        const loaderPromise = loader.load('./foo/bar/baz.mjs');
 
-          expect(requireShim).toHaveBeenCalledWith('./foo/bar/baz');
-          expect(importShim).not.toHaveBeenCalled();
-        });
+        expect(requireShim).not.toHaveBeenCalled();
+        expect(importShim).toHaveBeenCalledWith('file://./foo/bar/baz.mjs');
+        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}, false);
+        resolve();
 
-          await expectAsync(loader.load('foo')).toBeRejectedWith(underlyingError);
-        });
+        await expectAsync(loaderPromise).toBeResolved();
       });
-    });
-  });
-});
 
-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});
+      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});
 
-    const loaderPromise = loader.load(`./foo/bar/baz.${extension}`, alwaysImport);
+        await expectAsync(loader.load('foo.mjs')).toBeRejectedWithError(
+          "While loading foo.mjs: SyntaxError: some details but no filename, not even in the stack trace"
+        );
+      });
 
-    expect(requireShim).not.toHaveBeenCalled();
-    expect(importShim).toHaveBeenCalledWith(`file://./foo/bar/baz.${extension}`);
-    await expectAsync(loaderPromise).toBePending();
+      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});
 
-    resolve();
+        await expectAsync(loader.load('foo.mjs')).toBeRejectedWith(underlyingError);
+      });
+    });
 
-    await expectAsync(loaderPromise).toBeResolved();
-  });
+    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});
 
-  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/bar/baz')).toBeResolved();
 
-    await expectAsync(loader.load(`foo.${extension}`, alwaysImport)).toBeRejectedWithError(
-      `While loading foo.${extension}: SyntaxError: some details but no filename, not even in the stack trace`
-    );
-  });
+        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.${extension}`);
-    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});
 
-    await expectAsync(loader.load(`foo.${extension}`, alwaysImport)).toBeRejectedWith(underlyingError);
+        await expectAsync(loader.load('foo')).toBeRejectedWith(underlyingError);
+      });
+    });
   });
-}
+});