From bb9a194221136732a87bc7284b28bfe6982e3d97 Mon Sep 17 00:00:00 2001 From: David Goss Date: Mon, 4 Apr 2022 20:56:31 +0100 Subject: [PATCH 1/5] try require first when loadinh formatters --- src/formatter/builder.ts | 29 +++++++++++++-- src/formatter/builder_spec.ts | 70 ++++++++++++++--------------------- 2 files changed, 53 insertions(+), 46 deletions(-) diff --git a/src/formatter/builder.ts b/src/formatter/builder.ts index 3d906acd9..fd2324b4c 100644 --- a/src/formatter/builder.ts +++ b/src/formatter/builder.ts @@ -13,7 +13,7 @@ import { EventEmitter } from 'events' import EventDataCollector from './helpers/event_data_collector' import { Writable as WritableStream } from 'stream' import { SnippetInterface } from './step_definition_snippet_builder/snippet_syntax' -import { pathToFileURL } from 'url' +import { fileURLToPath, pathToFileURL } from 'url' import Formatters from './helpers/formatters' // eslint-disable-next-line @typescript-eslint/no-var-requires const { importer } = require('../importer') @@ -97,9 +97,13 @@ const FormatterBuilder = { descriptor: string, cwd: string ) { - let CustomClass = descriptor.startsWith(`.`) - ? await importer(pathToFileURL(path.resolve(cwd, descriptor))) - : await importer(descriptor) + let normalised: URL | string = descriptor + if (descriptor.startsWith('.')) { + normalised = pathToFileURL(path.resolve(cwd, descriptor)) + } else if (descriptor.startsWith('file://')) { + normalised = new URL(descriptor) + } + let CustomClass = await FormatterBuilder.loadFile(normalised) CustomClass = FormatterBuilder.resolveConstructor(CustomClass) if (doesHaveValue(CustomClass)) { return CustomClass @@ -110,6 +114,23 @@ const FormatterBuilder = { } }, + async loadFile(urlOrName: URL | string) { + let result + try { + // eslint-disable-next-line @typescript-eslint/no-var-requires + result = require(typeof urlOrName === 'string' + ? urlOrName + : fileURLToPath(urlOrName)) + } catch (error) { + if (error.code === 'ERR_REQUIRE_ESM') { + result = await importer(urlOrName) + } else { + throw error + } + } + return result + }, + resolveConstructor(ImportedCode: any) { if (doesNotHaveValue(ImportedCode)) { return null diff --git a/src/formatter/builder_spec.ts b/src/formatter/builder_spec.ts index a6b7ea54d..75a385720 100644 --- a/src/formatter/builder_spec.ts +++ b/src/formatter/builder_spec.ts @@ -4,46 +4,32 @@ import { pathToFileURL } from 'url' import path from 'path' describe('custom class loading', () => { - it('should handle a file:// url', async () => { - const fileUrl = pathToFileURL( - path.resolve(__dirname, './fixtures/module_dot_exports.cjs') - ).toString() - const CustomClass = await FormatterBuilder.loadCustomClass( - 'formatter', - fileUrl, - __dirname - ) - - expect(typeof CustomClass).to.eq('function') - }) - - it('should handle cjs module.exports', async () => { - const CustomClass = await FormatterBuilder.loadCustomClass( - 'formatter', - './fixtures/module_dot_exports.cjs', - __dirname - ) - - expect(typeof CustomClass).to.eq('function') - }) - - it('should handle cjs exports.default', async () => { - const CustomClass = await FormatterBuilder.loadCustomClass( - 'formatter', - './fixtures/exports_dot_default.cjs', - __dirname - ) - - expect(typeof CustomClass).to.eq('function') - }) - - it('should handle esm default export', async () => { - const CustomClass = await FormatterBuilder.loadCustomClass( - 'formatter', - './fixtures/esm.mjs', - __dirname - ) - - expect(typeof CustomClass).to.eq('function') - }) + ;['esm.mjs', 'exports_dot_default.cjs', 'module_dot_exports.cjs'].forEach( + (filename) => { + describe(filename, () => { + it('should handle a relative path', async () => { + const CustomClass = await FormatterBuilder.loadCustomClass( + 'formatter', + `./fixtures/${filename}`, + __dirname + ) + + expect(typeof CustomClass).to.eq('function') + }) + + it('should handle a file:// url', async () => { + const fileUrl = pathToFileURL( + path.resolve(__dirname, `./fixtures/${filename}`) + ).toString() + const CustomClass = await FormatterBuilder.loadCustomClass( + 'formatter', + fileUrl, + __dirname + ) + + expect(typeof CustomClass).to.eq('function') + }) + }) + } + ) }) From 558d5176f723252254f0e44a9a4655d3f7446422 Mon Sep 17 00:00:00 2001 From: David Goss Date: Mon, 4 Apr 2022 20:58:28 +0100 Subject: [PATCH 2/5] prove that transpiled ones work too --- src/formatter/builder_spec.ts | 52 +++++++++++++++------------- src/formatter/fixtures/typescript.ts | 1 + 2 files changed, 29 insertions(+), 24 deletions(-) create mode 100644 src/formatter/fixtures/typescript.ts diff --git a/src/formatter/builder_spec.ts b/src/formatter/builder_spec.ts index 75a385720..215575dfc 100644 --- a/src/formatter/builder_spec.ts +++ b/src/formatter/builder_spec.ts @@ -4,32 +4,36 @@ import { pathToFileURL } from 'url' import path from 'path' describe('custom class loading', () => { - ;['esm.mjs', 'exports_dot_default.cjs', 'module_dot_exports.cjs'].forEach( - (filename) => { - describe(filename, () => { - it('should handle a relative path', async () => { - const CustomClass = await FormatterBuilder.loadCustomClass( - 'formatter', - `./fixtures/${filename}`, - __dirname - ) + const varieties = [ + 'esm.mjs', + 'exports_dot_default.cjs', + 'module_dot_exports.cjs', + 'typescript.ts', + ] + varieties.forEach((filename) => { + describe(filename, () => { + it('should handle a relative path', async () => { + const CustomClass = await FormatterBuilder.loadCustomClass( + 'formatter', + `./fixtures/${filename}`, + __dirname + ) - expect(typeof CustomClass).to.eq('function') - }) + expect(typeof CustomClass).to.eq('function') + }) - it('should handle a file:// url', async () => { - const fileUrl = pathToFileURL( - path.resolve(__dirname, `./fixtures/${filename}`) - ).toString() - const CustomClass = await FormatterBuilder.loadCustomClass( - 'formatter', - fileUrl, - __dirname - ) + it('should handle a file:// url', async () => { + const fileUrl = pathToFileURL( + path.resolve(__dirname, `./fixtures/${filename}`) + ).toString() + const CustomClass = await FormatterBuilder.loadCustomClass( + 'formatter', + fileUrl, + __dirname + ) - expect(typeof CustomClass).to.eq('function') - }) + expect(typeof CustomClass).to.eq('function') }) - } - ) + }) + }) }) diff --git a/src/formatter/fixtures/typescript.ts b/src/formatter/fixtures/typescript.ts new file mode 100644 index 000000000..2174f613f --- /dev/null +++ b/src/formatter/fixtures/typescript.ts @@ -0,0 +1 @@ +export default class Formatter {} From 3c1b165cad080b5a2711ac379c000e10e9d2dce8 Mon Sep 17 00:00:00 2001 From: David Goss Date: Mon, 4 Apr 2022 21:03:19 +0100 Subject: [PATCH 3/5] simplify logic to resolve ctor --- src/formatter/builder.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/formatter/builder.ts b/src/formatter/builder.ts index fd2324b4c..ac0419efd 100644 --- a/src/formatter/builder.ts +++ b/src/formatter/builder.ts @@ -142,11 +142,6 @@ const FormatterBuilder = { typeof ImportedCode.default === 'function' ) { return ImportedCode.default - } else if ( - typeof ImportedCode.default === 'object' && - typeof ImportedCode.default.default === 'function' - ) { - return ImportedCode.default.default } return null }, From 93d50f4cd02af1a9e894fffe3f525d238d5a1405 Mon Sep 17 00:00:00 2001 From: David Goss Date: Mon, 4 Apr 2022 22:08:32 +0100 Subject: [PATCH 4/5] update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index dfe08b342..0d7c5dbfa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Please see [CONTRIBUTING.md](https://github.com/cucumber/cucumber/blob/master/CO ## [Unreleased] ### Fixed - Allow `file://` URLs to be used as formatter/snippet paths in options ([#1963](https://github.com/cucumber/cucumber-js/pull/1963) [#1920](https://github.com/cucumber/cucumber-js/issues/1920)) +- Allow custom formatters to rely on `--require-module` transpilers ([#1985](https://github.com/cucumber/cucumber-js/pull/1985)) ### Changed - Emit a warning when using a Node.js version that's untested with Cucumber ([#1959](https://github.com/cucumber/cucumber-js/pull/1959)) From fdedd7a176140b8b7cb426f23e678595c928c309 Mon Sep 17 00:00:00 2001 From: David Goss Date: Wed, 6 Apr 2022 09:06:00 +0100 Subject: [PATCH 5/5] extend feature test to prove transpiled formatters work --- features/custom_formatter.feature | 1 + 1 file changed, 1 insertion(+) diff --git a/features/custom_formatter.feature b/features/custom_formatter.feature index daff0b374..22cd33e7c 100644 --- a/features/custom_formatter.feature +++ b/features/custom_formatter.feature @@ -139,6 +139,7 @@ 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 |