Skip to content

Commit

Permalink
fix(import): wrong error message when a module exists but fails
Browse files Browse the repository at this point in the history
The importer is responsible of importing and patching modules with
configurable fallbacks. If one is missing, it goes to the next one.
Previously if a module existed but was failing when loading it with
`require`, it'd report as missing instead of failing. Now it bubbles the
error correctly.
  • Loading branch information
huafu committed Sep 26, 2018
1 parent e765875 commit e0d6c57
Show file tree
Hide file tree
Showing 5 changed files with 143 additions and 39 deletions.
2 changes: 1 addition & 1 deletion src/config/config-set.ts
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ export class ConfigSet {
let hooksFile = process.env.TS_JEST_HOOKS
if (hooksFile) {
hooksFile = resolve(this.cwd, hooksFile)
return importer.tryThese(hooksFile) || {}
return importer.tryTheseOr(hooksFile, {})
}
return {}
}
Expand Down
7 changes: 0 additions & 7 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,13 +126,6 @@ export interface CreateJestPresetOptions {
*/
export type ModulePatcher<T = any> = (module: T) => T

/**
* @internal
*/
export interface TsJestImporter {
tryThese(moduleName: string, ...fallbacks: string[]): any
}

/**
* Common TypeScript interfaces between versions.
*/
Expand Down
83 changes: 65 additions & 18 deletions src/util/importer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,21 @@ import * as fakers from '../__helpers__/fakers'

import { Importer, __requireModule } from './importer'

const requireModule = jest.fn(
mod =>
mod in modules
? modules[mod]()
: (() => {
const err: any = new Error(`Module not found: ${mod}.`)
err.code = 'MODULE_NOT_FOUND'
throw err
})(),
)
__requireModule(requireModule as any)
const moduleNotFound = (mod: string) => {
const err: any = new Error(`Module not found: ${mod}.`)
err.code = 'MODULE_NOT_FOUND'
throw err
}
const fakeFullPath = (mod: string) => `/root/${mod}.js`
const requireModule = jest.fn(mod => (mod in modules ? modules[mod]() : moduleNotFound(mod)))
const resolveModule = jest.fn(mod => (mod in modules ? fakeFullPath(mod) : moduleNotFound(mod)))
__requireModule(requireModule as any, resolveModule)

let modules!: { [key: string]: () => any }
beforeEach(() => {
modules = {}
requireModule.mockClear()
resolveModule.mockClear()
})

describe('instance', () => {
Expand All @@ -30,12 +29,60 @@ describe('instance', () => {
})
})

describe('tryTheese', () => {
it('tries until it find one not failing', () => {
describe('tryThese', () => {
it('should try until it finds one existing', () => {
modules = {
success: () => 'success',
success: () => 'ok',
}
expect(new Importer().tryThese('fail1', 'fail2', 'success')).toBe('success')
expect(new Importer().tryThese('missing1', 'missing2', 'success')).toMatchInlineSnapshot(`
Object {
"exists": true,
"exports": "ok",
"given": "success",
"path": "/root/success.js",
}
`)
})
it('should return the error when one is failing', () => {
modules = {
fail1: () => {
throw new Error('foo')
},
success: () => 'ok',
}
const res = new Importer().tryThese('missing1', 'fail1', 'success')
expect(res).toMatchObject({
exists: true,
error: expect.any(Error),
given: 'fail1',
path: fakeFullPath('fail1'),
})
expect(res).not.toHaveProperty('exports')
expect((res as any).error.message).toMatch(/\bfoo\b/)
})
})

describe('tryTheseOr', () => {
it('should try until it find one not failing', () => {
expect(new Importer().tryTheseOr(['fail1', 'fail2', 'success'])).toBeUndefined()
expect(new Importer().tryTheseOr(['fail1', 'fail2', 'success'], 'foo')).toBe('foo')
modules = {
success: () => 'ok',
}
expect(new Importer().tryTheseOr(['fail1', 'fail2', 'success'])).toBe('ok')
modules.fail2 = () => {
throw new Error('foo')
}
expect(new Importer().tryTheseOr(['fail1', 'fail2', 'success'], 'bar', true)).toBe('bar')
})
it('should fail if one is throwing', () => {
modules = {
success: () => 'ok',
fail2: () => {
throw new Error('foo')
},
}
expect(() => new Importer().tryTheseOr(['fail1', 'fail2', 'success'], 'bar')).toThrow(/\bfoo\b/)
})
})

Expand All @@ -49,10 +96,10 @@ describe('patcher', () => {
foo: () => ({ foo: true }),
bar: () => ({ bar: true }),
}
expect(imp.tryThese('foo')).toEqual({ foo: true, p1: true, p2: true })
expect(imp.tryThese('foo')).toEqual({ foo: true, p1: true, p2: true })
expect(imp.tryTheseOr('foo')).toEqual({ foo: true, p1: true, p2: true })
expect(imp.tryTheseOr('foo')).toEqual({ foo: true, p1: true, p2: true })

expect(imp.tryThese('bar')).toEqual({ bar: true })
expect(imp.tryTheseOr('bar')).toEqual({ bar: true })

// ensure cache has been used
expect(patch1).toHaveBeenCalledTimes(1)
Expand Down
89 changes: 76 additions & 13 deletions src/util/importer.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ModulePatcher, TBabelCore, TBabelJest, TTypeScript, TsJestImporter } from '../types'
import { ModulePatcher, TBabelCore, TBabelJest, TTypeScript } from '../types'

import * as hacks from './hacks'
import { rootLogger } from './logger'
Expand Down Expand Up @@ -26,7 +26,7 @@ const passThru = (action: () => void) => (input: any) => {
/**
* @internal
*/
export class Importer implements TsJestImporter {
export class Importer {
@Memoize()
static get instance() {
logger.debug('creating Importer singleton')
Expand Down Expand Up @@ -70,22 +70,51 @@ export class Importer implements TsJestImporter {
}

@Memoize((...args: string[]) => args.join(':'))
tryThese(moduleName: string, ...fallbacks: string[]): any {
tryThese(moduleName: string, ...fallbacks: string[]) {
let name: string
let loaded: any
let loaded: RequireResult<true> | undefined
const tries = [moduleName, ...fallbacks]
// tslint:disable-next-line:no-conditional-assignment
while ((name = tries.shift() as string) !== undefined) {
try {
loaded = requireModule(name)
logger.debug('loaded module', name)
const req = requireWrapper(name)

// remove exports from what we're going to log
const contextReq: any = { ...req }
delete contextReq.exports

if (req.exists) {
// module exists
loaded = req as RequireResult<true>
if (loaded.error) {
// require-ing it failed
logger.error({ requireResult: contextReq }, `failed loading module '${name}'`, loaded.error.message)
} else {
// it has been loaded, let's patch it
logger.debug({ requireResult: contextReq }, 'loaded module', name)
loaded.exports = this._patch(name, loaded.exports)
}
break
} catch (err) {
logger.debug('fail loading module', name)
} else {
// module does not exists in the path
logger.debug({ requireResult: contextReq }, `module '${name}' not found`)
continue
}
}

return loaded && this._patch(name, loaded)
// return the loaded one, could be one that has been loaded, or one which has failed during load
// but not one which does not exists
return loaded
}

tryTheseOr<T>(moduleNames: [string, ...string[]] | string, missingResult: T, allowLoadError?: boolean): T
tryTheseOr<T>(moduleNames: [string, ...string[]] | string, missingResult?: T, allowLoadError?: boolean): T | undefined
tryTheseOr<T>(moduleNames: [string, ...string[]] | string, missingResult?: T, allowLoadError = false): T | undefined {
const args: [string, ...string[]] = Array.isArray(moduleNames) ? moduleNames : [moduleNames]
const result = this.tryThese(...args)
if (!result) return missingResult
if (!result.error) return result.exports as T
if (allowLoadError) return missingResult
throw result.error
}

@Memoize(name => name)
Expand All @@ -105,8 +134,10 @@ export class Importer implements TsJestImporter {
// try to load any of the alternative after trying main one
const res = this.tryThese(moduleName, ...alternatives)
// if we could load one, return it
if (res) {
return res
if (res && res.exists) {
if (!res.error) return res.exports
// it could not load because of a failure while importing, but it exists
throw new Error(interpolate(Errors.LoadingModuleFailed, { module: res.given, error: res.error.message }))
}

// if it couldn't load, build a nice error message so the user can fix it by himself
Expand Down Expand Up @@ -136,11 +167,43 @@ export class Importer implements TsJestImporter {
*/
export const importer = Importer.instance

/**
* @internal
*/
export interface RequireResult<E = boolean> {
exists: E
given: string
path?: string
exports?: any
error?: Error
}

function requireWrapper(moduleName: string): RequireResult {
let path: string
let exists = false
try {
path = resolveModule(moduleName)
exists = true
} catch (error) {
return { error, exists, given: moduleName }
}
const result: RequireResult = { exists, path, given: moduleName }
try {
result.exports = requireModule(moduleName)
} catch (error) {
result.error = error
}
return result
}

let requireModule = (mod: string) => require(mod)
let resolveModule = (mod: string) => require.resolve(mod)

/**
* @internal
*/
// so that we can test easier
export function __requireModule(localRequire: typeof requireModule) {
export function __requireModule(localRequire: typeof requireModule, localResolve: typeof resolveModule) {
requireModule = localRequire
resolveModule = localResolve
}
1 change: 1 addition & 0 deletions src/util/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* @internal
*/
export enum Errors {
LoadingModuleFailed = 'Loading module {{module}} failed with error: {{error}}',
UnableToLoadOneModule = 'Unable to load the module {{module}}. {{reason}} To fix it:\n{{fix}}',
UnableToLoadAnyModule = 'Unable to load any of these modules: {{module}}. {{reason}}. To fix it:\n{{fix}}',
TypesUnavailableWithoutTypeCheck = 'Type information is unavailable with "isolatedModules"',
Expand Down

0 comments on commit e0d6c57

Please sign in to comment.