use import wherever possible to load code (#2334)

CHANGELOG.md

@@ -11,6 +11,10 @@ Please see [CONTRIBUTING.md](./CONTRIBUTING.md) on how to contribute to Cucumber
 ### Added
 - Add support for Node.js 20 ([#2331](https://github.com/cucumber/cucumber-js/pull/2331))
 
+### Changed
+- BREAKING CHANGE: Use appropriate module loading mechanism for configuration files ([#2334](https://github.com/cucumber/cucumber-js/pull/2334))
+- BREAKING CHANGE: Use `await import()` to load all custom formatters and snippet syntaxes ([#2334](https://github.com/cucumber/cucumber-js/pull/2334))
+
 ### Removed
 - BREAKING CHANGE: Drop support for Node.js 14, 16 and 19 ([#2331](https://github.com/cucumber/cucumber-js/pull/2331))
 

dependency-lint.yml

@@ -48,7 +48,6 @@ requiredModules:
       - 'dist/**/*'
       - 'lib/**/*'
       - 'node_modules/**/*'
-      - 'src/importer.js'
       - 'tmp/**/*'
     root: '**/*.{js,ts}'
   stripLoaders: false

features/custom_formatter.feature

@@ -30,9 +30,9 @@ Feature: custom formatter
           this.log(testCaseAttempt.gherkinDocument.feature.name + ' / ' + testCaseAttempt.pickle.name + '\n')
           const parsed = formatterHelpers.parseTestCaseAttempt({
             cwd: this.cwd,
-            snippetBuilder: this.snippetBuilder, 
+            snippetBuilder: this.snippetBuilder,
             supportCodeLibrary: this.supportCodeLibrary,
-            testCaseAttempt 
+            testCaseAttempt
           })
           parsed.testSteps.forEach(testStep => {
             this.log('  ' + testStep.keyword + (testStep.text || '') + ' - ' + Status[testStep.result.status] + '\n')
@@ -83,9 +83,9 @@ Feature: custom formatter
           this.log(testCaseAttempt.gherkinDocument.feature.name + ' / ' + testCaseAttempt.pickle.name + '\n')
           const parsed = formatterHelpers.parseTestCaseAttempt({
             cwd: this.cwd,
-            snippetBuilder: this.snippetBuilder, 
+            snippetBuilder: this.snippetBuilder,
             supportCodeLibrary: this.supportCodeLibrary,
-            testCaseAttempt 
+            testCaseAttempt
           })
           parsed.testSteps.forEach(testStep => {
             this.log('  ' + testStep.keyword + (testStep.text || '') + ' - ' + Status[testStep.result.status] + '\n')
@@ -139,7 +139,6 @@ Feature: custom formatter
     Then it passes
     Examples:
       | EXT  | IMPORT_STATEMENT                                  | EXPORT_STATEMENT                  |
-      | .ts  | import {Formatter} from '@cucumber/cucumber'      | export default CustomFormatter    |
       | .mjs | import {Formatter} from '@cucumber/cucumber'      | export default CustomFormatter    |
       | .js  | const {Formatter} = require('@cucumber/cucumber') | module.exports = CustomFormatter  |
       | .js  | const {Formatter} = require('@cucumber/cucumber') | exports.default = CustomFormatter |

package.json

@@ -236,6 +236,7 @@
     "mkdirp": "^2.1.5",
     "mz": "^2.7.0",
     "progress": "^2.0.3",
+    "read-pkg-up": "^7.0.1",
     "resolve-pkg": "^2.0.0",
     "semver": "7.5.3",
     "string-argv": "^0.3.1",
@@ -310,7 +311,7 @@
     "ansi-regex": "^5.0.1"
   },
   "scripts": {
-    "build-local": "genversion --es6 src/version.ts && tsc --build tsconfig.node.json && shx cp src/importer.js lib/ && shx cp src/wrapper.mjs lib/ && shx cp src/api/wrapper.mjs lib/api/",
+    "build-local": "genversion --es6 src/version.ts && tsc --build tsconfig.node.json && shx cp src/wrapper.mjs lib/ && shx cp src/api/wrapper.mjs lib/api/",
     "cck-test": "mocha 'compatibility/**/*_spec.ts'",
     "docs:ci": "api-extractor run --verbose",
     "docs:local": "api-extractor run --verbose --local && api-documenter markdown --input-folder ./tmp/api-extractor --output-folder ./docs/api",

src/api/support.ts

@@ -4,9 +4,6 @@ import supportCodeLibraryBuilder from '../support_code_library_builder'
 import { pathToFileURL } from 'url'
 import tryRequire from '../try_require'
 
-// eslint-disable-next-line @typescript-eslint/no-var-requires
-const { importer } = require('../importer')
-
 export async function getSupportCodeLibrary({
   cwd,
   newId,
@@ -30,7 +27,7 @@ export async function getSupportCodeLibrary({
   requirePaths.map((path) => tryRequire(path))
 
   for (const path of importPaths) {
-    await importer(pathToFileURL(path))
+    await import(pathToFileURL(path).toString())
   }
 
   return supportCodeLibraryBuilder.finalize()

src/configuration/from_file.ts

@@ -2,6 +2,7 @@ import stringArgv from 'string-argv'
 import fs from 'fs'
 import path from 'path'
 import YAML from 'yaml'
+import readPkgUp from 'read-pkg-up'
 import { promisify } from 'util'
 import { pathToFileURL } from 'url'
 import { IConfiguration } from './types'
@@ -10,16 +11,13 @@ import ArgvParser from './argv_parser'
 import { checkSchema } from './check_schema'
 import { ILogger } from '../logger'
 
-// eslint-disable-next-line @typescript-eslint/no-var-requires
-const { importer } = require('../importer')
-
 export async function fromFile(
   logger: ILogger,
   cwd: string,
   file: string,
   profiles: string[] = []
 ): Promise<Partial<IConfiguration>> {
-  const definitions = await loadFile(cwd, file)
+  const definitions = await loadFile(logger, cwd, file)
   if (!definitions.default) {
     logger.debug('No default profile defined in configuration file')
     definitions.default = {}
@@ -43,6 +41,7 @@ export async function fromFile(
 }
 
 async function loadFile(
+  logger: ILogger,
   cwd: string,
   file: string
 ): Promise<Record<string, any>> {
@@ -61,24 +60,56 @@ async function loadFile(
         await promisify(fs.readFile)(filePath, { encoding: 'utf-8' })
       )
       break
-    default:
-      try {
-        // eslint-disable-next-line @typescript-eslint/no-var-requires
-        definitions = require(filePath)
-      } catch (error) {
-        if (error.code === 'ERR_REQUIRE_ESM') {
-          definitions = await importer(pathToFileURL(filePath))
+    case '.cjs':
+      logger.debug(
+        `Loading configuration file "${file}" as CommonJS based on extension`
+      )
+      // eslint-disable-next-line @typescript-eslint/no-var-requires
+      definitions = require(filePath)
+      break
+    case '.mjs':
+      logger.debug(
+        `Loading configuration file "${file}" as ESM based on extension`
+      )
+      definitions = await import(pathToFileURL(filePath).toString())
+      break
+    case '.js':
+      {
+        const parentPackage = await readPackageJson(filePath)
+        if (!parentPackage) {
+          logger.debug(
+            `Loading configuration file "${file}" as CommonJS based on absence of a parent package`
+          )
+          // eslint-disable-next-line @typescript-eslint/no-var-requires
+          definitions = require(filePath)
+        } else if (parentPackage.type === 'module') {
+          logger.debug(
+            `Loading configuration file "${file}" as ESM based on "${parentPackage.name}" package type`
+          )
+          definitions = await import(pathToFileURL(filePath).toString())
         } else {
-          throw error
+          logger.debug(
+            `Loading configuration file "${file}" as CommonJS based on "${parentPackage.name}" package type`
+          )
+          // eslint-disable-next-line @typescript-eslint/no-var-requires
+          definitions = require(filePath)
         }
       }
+      break
+    default:
+      throw new Error(`Unsupported configuration file extension "${extension}"`)
   }
   if (typeof definitions !== 'object') {
     throw new Error(`Configuration file ${filePath} does not export an object`)
   }
   return definitions
 }
 
+async function readPackageJson(filePath: string) {
+  const parentPackage = await readPkgUp({ cwd: path.dirname(filePath) })
+  return parentPackage?.packageJson
+}
+
 function extractConfiguration(
   logger: ILogger,
   name: string,

src/configuration/from_file_spec.ts

@@ -7,40 +7,48 @@ import { fromFile } from './from_file'
 import { FakeLogger } from '../../test/fake_logger'
 
 async function setup(
-  file: string = 'cucumber.js',
-  content: string = `module.exports = {default: {paths: ['some/path/*.feature']}, p1: {paths: ['other/path/*.feature']}, p2: 'other/other/path/*.feature --no-strict'}`
+  file: string = 'cucumber.json',
+  content: string = JSON.stringify({
+    default: { paths: ['some/path/*.feature'] },
+    p1: { paths: ['other/path/*.feature'] },
+    p2: 'other/other/path/*.feature --no-strict',
+  }),
+  packageJson: string = `{}`
 ) {
   const logger = new FakeLogger()
   const cwd = await promisify<DirOptions, string>(tmp.dir)({
     unsafeCleanup: true,
   })
   fs.writeFileSync(path.join(cwd, file), content, { encoding: 'utf-8' })
+  fs.writeFileSync(path.join(cwd, 'package.json'), packageJson, {
+    encoding: 'utf-8',
+  })
   return { logger, cwd }
 }
 
 describe('fromFile', () => {
   it('should return empty config if no default provide and no profiles requested', async () => {
     const { logger, cwd } = await setup(
-      'cucumber.js',
-      `module.exports = {p1: {paths: ['other/path/*.feature']}}`
+      'cucumber.json',
+      JSON.stringify({ p1: { paths: ['other/path/*.feature'] } })
     )
 
-    const result = await fromFile(logger, cwd, 'cucumber.js', [])
+    const result = await fromFile(logger, cwd, 'cucumber.json', [])
     expect(result).to.deep.eq({})
   })
 
   it('should get default config from file if no profiles requested', async () => {
     const { logger, cwd } = await setup()
 
-    const result = await fromFile(logger, cwd, 'cucumber.js', [])
+    const result = await fromFile(logger, cwd, 'cucumber.json', [])
     expect(result).to.deep.eq({ paths: ['some/path/*.feature'] })
   })
 
   it('should throw when a requested profile doesnt exist', async () => {
     const { logger, cwd } = await setup()
 
     try {
-      await fromFile(logger, cwd, 'cucumber.js', ['nope'])
+      await fromFile(logger, cwd, 'cucumber.json', ['nope'])
       expect.fail('should have thrown')
     } catch (error) {
       expect(error.message).to.eq(`Requested profile "nope" doesn't exist`)
@@ -50,14 +58,14 @@ describe('fromFile', () => {
   it('should get single profile config from file', async () => {
     const { logger, cwd } = await setup()
 
-    const result = await fromFile(logger, cwd, 'cucumber.js', ['p1'])
+    const result = await fromFile(logger, cwd, 'cucumber.json', ['p1'])
     expect(result).to.deep.eq({ paths: ['other/path/*.feature'] })
   })
 
   it('should merge multiple profiles config from file', async () => {
     const { logger, cwd } = await setup()
 
-    const result = await fromFile(logger, cwd, 'cucumber.js', ['p1', 'p2'])
+    const result = await fromFile(logger, cwd, 'cucumber.json', ['p1', 'p2'])
     expect(result).to.deep.eq({
       paths: ['other/path/*.feature', 'other/other/path/*.feature'],
       strict: false,
@@ -66,11 +74,11 @@ describe('fromFile', () => {
 
   it('should throw when an object doesnt conform to the schema', async () => {
     const { logger, cwd } = await setup(
-      'cucumber.js',
-      `module.exports = {p1: {paths: 4, things: 8, requireModule: 'aardvark'}}`
+      'cucumber.json',
+      JSON.stringify({ p1: { paths: 4, things: 8, requireModule: 'aardvark' } })
     )
     try {
-      await fromFile(logger, cwd, 'cucumber.js', ['p1'])
+      await fromFile(logger, cwd, 'cucumber.json', ['p1'])
       expect.fail('should have thrown')
     } catch (error) {
       expect(error.message).to.eq(
@@ -79,8 +87,8 @@ describe('fromFile', () => {
     }
   })
 
-  describe('other formats', () => {
-    it('should work with esm', async () => {
+  describe('supported formats', () => {
+    it('should work with .mjs', async () => {
       const { logger, cwd } = await setup(
         'cucumber.mjs',
         `export default {}; export const p1 = {paths: ['other/path/*.feature']}`
@@ -90,7 +98,49 @@ describe('fromFile', () => {
       expect(result).to.deep.eq({ paths: ['other/path/*.feature'] })
     })
 
-    it('should work with json', async () => {
+    it('should work with .cjs', async () => {
+      const { logger, cwd } = await setup(
+        'cucumber.cjs',
+        `module.exports = { default: {}, p1: { paths: ['other/path/*.feature'] } }`
+      )
+
+      const result = await fromFile(logger, cwd, 'cucumber.cjs', ['p1'])
+      expect(result).to.deep.eq({ paths: ['other/path/*.feature'] })
+    })
+
+    it('should work with .js when commonjs (undeclared)', async () => {
+      const { logger, cwd } = await setup(
+        'cucumber.js',
+        `module.exports = { default: {}, p1: { paths: ['other/path/*.feature'] } }`
+      )
+
+      const result = await fromFile(logger, cwd, 'cucumber.js', ['p1'])
+      expect(result).to.deep.eq({ paths: ['other/path/*.feature'] })
+    })
+
+    it('should work with .js when commonjs (explicit)', async () => {
+      const { logger, cwd } = await setup(
+        'cucumber.js',
+        `module.exports = { default: {}, p1: { paths: ['other/path/*.feature'] } }`,
+        JSON.stringify({ type: 'commonjs' })
+      )
+
+      const result = await fromFile(logger, cwd, 'cucumber.js', ['p1'])
+      expect(result).to.deep.eq({ paths: ['other/path/*.feature'] })
+    })
+
+    it('should work with .js when module (explicit)', async () => {
+      const { logger, cwd } = await setup(
+        'cucumber.js',
+        `export default {}; export const p1 = {paths: ['other/path/*.feature']}`,
+        JSON.stringify({ type: 'module' })
+      )
+
+      const result = await fromFile(logger, cwd, 'cucumber.js', ['p1'])
+      expect(result).to.deep.eq({ paths: ['other/path/*.feature'] })
+    })
+
+    it('should work with .json', async () => {
       const { logger, cwd } = await setup(
         'cucumber.json',
         `{ "default": {}, "p1": { "paths": ["other/path/*.feature"] } }`
@@ -100,7 +150,7 @@ describe('fromFile', () => {
       expect(result).to.deep.eq({ paths: ['other/path/*.feature'] })
     })
 
-    it('should work with yaml', async () => {
+    it('should work with .yaml', async () => {
       const { logger, cwd } = await setup(
         'cucumber.yaml',
         `default:
@@ -114,5 +164,32 @@ p1:
       const result = await fromFile(logger, cwd, 'cucumber.yaml', ['p1'])
       expect(result).to.deep.eq({ paths: ['other/path/*.feature'] })
     })
+
+    it('should work with .yml', async () => {
+      const { logger, cwd } = await setup(
+        'cucumber.yml',
+        `default:
+
+p1:
+  paths:
+    - "other/path/*.feature"
+`
+      )
+
+      const result = await fromFile(logger, cwd, 'cucumber.yml', ['p1'])
+      expect(result).to.deep.eq({ paths: ['other/path/*.feature'] })
+    })
+
+    it('should throw for an unsupported format', async () => {
+      const { logger, cwd } = await setup('cucumber.foo', `{}`)
+      try {
+        await fromFile(logger, cwd, 'cucumber.foo', ['p1'])
+        expect.fail('should have thrown')
+      } catch (error) {
+        expect(error.message).to.eq(
+          'Unsupported configuration file extension ".foo"'
+        )
+      }
+    })
   })
 })