Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: try require first for custom formatters so transpiled ones work #1985

Merged
merged 5 commits into from Apr 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 {}