Skip to content

Commit

Permalink
fix: try require first for custom formatters so transpiled ones work (#…
Browse files Browse the repository at this point in the history
…1985)

* try require first when loadinh formatters

* prove that transpiled ones work too

* simplify logic to resolve ctor

* update CHANGELOG.md

* extend feature test to prove transpiled formatters work
  • Loading branch information
davidjgoss committed Apr 6, 2022
1 parent 901561e commit c97252d
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 50 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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))
Expand Down
1 change: 1 addition & 0 deletions features/custom_formatter.feature
Expand Up @@ -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 |
34 changes: 25 additions & 9 deletions src/formatter/builder.ts
Expand Up @@ -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')
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -121,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
},
Expand Down
72 changes: 31 additions & 41 deletions src/formatter/builder_spec.ts
Expand Up @@ -4,46 +4,36 @@ 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')
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')
})

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')
})
})
})
})
1 change: 1 addition & 0 deletions src/formatter/fixtures/typescript.ts
@@ -0,0 +1 @@
export default class Formatter {}

0 comments on commit c97252d

Please sign in to comment.