From 4f0bb338fc5495c2fb1046cecdb77f80e036cc2e Mon Sep 17 00:00:00 2001 From: Ahn Date: Wed, 8 Apr 2020 07:50:10 +0200 Subject: [PATCH] fix(config): use original jest config object instead of stringified config (#1511) --- .../__snapshots__/diagnostics.test.ts.snap | 4 +- src/__helpers__/fakers.ts | 7 +- src/compiler/compiler-utils.ts | 7 ++ src/compiler/language-service.spec.ts | 39 +++++++++ src/compiler/language-service.ts | 6 +- src/compiler/program.spec.ts | 79 +++++++++++++++++++ src/compiler/program.ts | 6 +- src/config/config-set.spec.ts | 52 ++++++------ src/config/config-set.ts | 5 +- src/ts-jest-transformer.spec.ts | 12 +-- src/ts-jest-transformer.ts | 49 ++++++------ 11 files changed, 191 insertions(+), 75 deletions(-) diff --git a/e2e/__tests__/__snapshots__/diagnostics.test.ts.snap b/e2e/__tests__/__snapshots__/diagnostics.test.ts.snap index d3c0933849..2f5427e2ad 100644 --- a/e2e/__tests__/__snapshots__/diagnostics.test.ts.snap +++ b/e2e/__tests__/__snapshots__/diagnostics.test.ts.snap @@ -242,7 +242,7 @@ exports[`With diagnostics throw using incremental program with unsupported versi TypeError: ts.createIncrementalCompilerHost is not a function - at Object.exports.compileUsingProgram (../../__templates__/with-typescript-2-7/node_modules/ts-jest/dist/compiler/program.js:47:19) + at Object.exports.compileUsingProgram (../../__templates__/with-typescript-2-7/node_modules/ts-jest/dist/compiler/program.js:46:19) Test Suites: 1 failed, 1 total Tests: 0 total @@ -264,7 +264,7 @@ exports[`With diagnostics throw using incremental program with unsupported versi TypeError: ts.createIncrementalCompilerHost is not a function - at Object.exports.compileUsingProgram (../../__templates__/with-unsupported-version/node_modules/ts-jest/dist/compiler/program.js:47:19) + at Object.exports.compileUsingProgram (../../__templates__/with-unsupported-version/node_modules/ts-jest/dist/compiler/program.js:46:19) Test Suites: 1 failed, 1 total Tests: 0 total diff --git a/src/__helpers__/fakers.ts b/src/__helpers__/fakers.ts index fcac848db3..a591a05da6 100644 --- a/src/__helpers__/fakers.ts +++ b/src/__helpers__/fakers.ts @@ -69,7 +69,12 @@ export function makeCompiler({ ...(tsJestConfig.diagnostics as any), pretty: false, } - jestConfig = { ...jestConfig, testMatch: ['^.+\\.tsx?$'], testRegex: ['^.+\\.[tj]sx?$'] } + const testRegex = ['^.+\\.[tj]sx?$'] + jestConfig = { + ...jestConfig, + testMatch: ['^.+\\.tsx?$'], + testRegex: jestConfig?.testRegex ? testRegex.concat(jestConfig.testRegex) : testRegex, + } const cs = new ConfigSet(getJestConfig(jestConfig, tsJestConfig), parentConfig) return createCompiler(cs) diff --git a/src/compiler/compiler-utils.ts b/src/compiler/compiler-utils.ts index 7254d4e141..ea98707baa 100644 --- a/src/compiler/compiler-utils.ts +++ b/src/compiler/compiler-utils.ts @@ -1,5 +1,6 @@ import { Logger } from 'bs-logger' import { writeFileSync } from 'fs' +import micromatch = require('micromatch') import { join, normalize } from 'path' import * as _ts from 'typescript' @@ -51,3 +52,9 @@ export function cacheResolvedModules( writeFileSync(getResolvedModulesCache(cacheDir), JSON.stringify(memoryCache.resolvedModules)) } } + +export function isTestFile(testMatchPatterns: [string, RegExp], fileName: string) { + return testMatchPatterns.some(pattern => + typeof pattern === 'string' ? micromatch.isMatch(fileName, pattern) : pattern.test(fileName), + ) +} diff --git a/src/compiler/language-service.spec.ts b/src/compiler/language-service.spec.ts index 7088afc67b..fb2550e05f 100644 --- a/src/compiler/language-service.spec.ts +++ b/src/compiler/language-service.spec.ts @@ -1,11 +1,14 @@ import { LogLevels } from 'bs-logger' import { removeSync, writeFileSync } from 'fs-extra' +import { normalize } from 'path' import { makeCompiler } from '../__helpers__/fakers' import { logTargetMock } from '../__helpers__/mocks' import { tempDir } from '../__helpers__/path' import ProcessedSource from '../__helpers__/processed-source' +import * as compilerUtils from './compiler-utils' + const logTarget = logTargetMock() describe('language service', () => { @@ -60,6 +63,42 @@ describe('language service', () => { removeSync(fileName) }) + it('should cache resolved modules for test file with testMatchPatterns from jest config when match', () => { + // tslint:disable-next-line:no-empty + const spy = jest.spyOn(compilerUtils, 'cacheResolvedModules').mockImplementationOnce(() => {}) + const tmp = tempDir('compiler') + const compiler = makeCompiler({ + jestConfig: { cache: true, cacheDirectory: tmp, testRegex: [/.*\.(spec|test)\.[jt]sx?$/] as any[] }, + tsJestConfig: { tsConfig: false }, + }) + const fileName = 'src/__mocks__/main.spec.ts' + const source = JSON.stringify(require('../__mocks__/main.spec')) + + compiler.compile(source, fileName) + + expect(spy).toHaveBeenCalled() + expect(spy.mock.calls[0][0]).toEqual(normalize(fileName)) + expect(spy.mock.calls[0][1]).toEqual(source) + + spy.mockRestore() + }) + + it(`shouldn't cache resolved modules for test file with testMatchPatterns from jest config when not match`, () => { + // tslint:disable-next-line:no-empty + jest.spyOn(compilerUtils, 'cacheResolvedModules').mockImplementationOnce(() => {}) + const tmp = tempDir('compiler') + const compiler = makeCompiler({ + jestConfig: { cache: true, cacheDirectory: tmp, testRegex: [/.*\.(foo|bar)\.[jt]sx?$/] as any[] }, + tsJestConfig: { tsConfig: false }, + }) + const fileName = 'src/__mocks__/main.spec.ts' + const source = JSON.stringify(require('../__mocks__/main.spec')) + + compiler.compile(source, fileName) + + expect(compilerUtils.cacheResolvedModules).not.toHaveBeenCalled() + }) + it('should compile js file for allowJs true with outDir', () => { const fileName = `test-allow-js-with-outDir.js` const compiler = makeCompiler({ diff --git a/src/compiler/language-service.ts b/src/compiler/language-service.ts index 13aba246f4..fbe10bcb7c 100644 --- a/src/compiler/language-service.ts +++ b/src/compiler/language-service.ts @@ -1,6 +1,5 @@ import { LogContexts, LogLevels, Logger } from 'bs-logger' import memoize = require('lodash.memoize') -import micromatch = require('micromatch') import { basename, normalize, relative } from 'path' import * as _ts from 'typescript' @@ -8,7 +7,7 @@ import { ConfigSet } from '../config/config-set' import { CompilerInstance, MemoryCache, SourceOutput } from '../types' import { Errors, interpolate } from '../util/messages' -import { cacheResolvedModules, hasOwn } from './compiler-utils' +import { cacheResolvedModules, hasOwn, isTestFile } from './compiler-utils' function doTypeChecking(configs: ConfigSet, fileName: string, service: _ts.LanguageService, logger: Logger) { if (configs.shouldReportDiagnostic(fileName)) { @@ -120,9 +119,8 @@ export const compileUsingLanguageService = ( /** * We don't need the following logic with no cache run because no cache always gives correct typing */ - /* istanbul ignore next (covered by e2e) */ if (cacheDir) { - if (micromatch.isMatch(normalizedFileName, configs.testMatchPatterns)) { + if (isTestFile(configs.testMatchPatterns, normalizedFileName)) { cacheResolvedModules(normalizedFileName, code, memoryCache, service.getProgram()!, cacheDir, logger) } else { /* istanbul ignore next (covered by e2e) */ diff --git a/src/compiler/program.spec.ts b/src/compiler/program.spec.ts index fff2b56153..f7854ba447 100644 --- a/src/compiler/program.spec.ts +++ b/src/compiler/program.spec.ts @@ -1,12 +1,15 @@ import { LogLevels } from 'bs-logger' import { writeFileSync } from 'fs' import { removeSync } from 'fs-extra' +import { normalize } from 'path' import { makeCompiler } from '../__helpers__/fakers' import { logTargetMock } from '../__helpers__/mocks' import { tempDir } from '../__helpers__/path' import ProcessedSource from '../__helpers__/processed-source' +import * as compilerUtils from './compiler-utils' + const logTarget = logTargetMock() const baseTsJestConfig = { @@ -107,6 +110,82 @@ describe('cache', () => { }) }) +describe('cache resolved modules for test file', () => { + let spy: jest.SpyInstance + const fileName = 'src/__mocks__/main.spec.ts' + const source = JSON.stringify(require('../__mocks__/main.spec')) + + describe('with program', () => { + beforeEach(() => { + // tslint:disable-next-line:no-empty + spy = jest.spyOn(compilerUtils, 'cacheResolvedModules').mockImplementationOnce(() => {}) + }) + + afterEach(() => { + spy.mockRestore() + }) + + it('should cache resolved modules for test file with testMatchPatterns from jest config when match', () => { + const tmp = tempDir('compiler') + const compiler = makeCompiler({ + jestConfig: { cache: true, cacheDirectory: tmp, testRegex: [/.*\.(spec|test)\.[jt]sx?$/] as any[] }, + tsJestConfig: { tsConfig: false, compilerHost: true, incremental: false }, + }) + compiler.compile(source, fileName) + + expect(spy).toHaveBeenCalled() + expect(spy.mock.calls[0][0]).toEqual(normalize(fileName)) + expect(spy.mock.calls[0][1]).toEqual(source) + }) + + it(`shouldn't cache resolved modules for test file with testMatchPatterns from jest config when not match`, () => { + const tmp = tempDir('compiler') + const compiler = makeCompiler({ + jestConfig: { cache: true, cacheDirectory: tmp, testRegex: [/.*\.(foo|bar)\.[jt]sx?$/] as any[] }, + tsJestConfig: { tsConfig: false, compilerHost: true, incremental: false }, + }) + compiler.compile(source, fileName) + + expect(spy).not.toHaveBeenCalled() + }) + }) + + describe('with incremental program', () => { + beforeEach(() => { + // tslint:disable-next-line:no-empty + spy = jest.spyOn(compilerUtils, 'cacheResolvedModules').mockImplementationOnce(() => {}) + }) + + afterEach(() => { + spy.mockRestore() + }) + + it('should cache resolved modules for test file with testMatchPatterns from jest config when match', () => { + const tmp = tempDir('compiler') + const compiler = makeCompiler({ + jestConfig: { cache: true, cacheDirectory: tmp, testRegex: [/.*\.(spec|test)\.[jt]sx?$/] as any[] }, + tsJestConfig: { tsConfig: false, compilerHost: true, incremental: true }, + }) + compiler.compile(source, fileName) + + expect(spy).toHaveBeenCalled() + expect(spy.mock.calls[0][0]).toEqual(normalize(fileName)) + expect(spy.mock.calls[0][1]).toEqual(source) + }) + + it(`shouldn't cache resolved modules for test file with testMatchPatterns from jest config when not match`, () => { + const tmp = tempDir('compiler') + const compiler = makeCompiler({ + jestConfig: { cache: true, cacheDirectory: tmp, testRegex: [/.*\.(foo|bar)\.[jt]sx?$/] as any[] }, + tsJestConfig: { tsConfig: false, compilerHost: true, incremental: true }, + }) + compiler.compile(source, fileName) + + expect(spy).not.toHaveBeenCalled() + }) + }) +}) + describe('allowJs', () => { const baseFileName = 'test-allowJs' const baseFileExt = 'test.js' diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 7abc674757..1199408a3e 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -1,6 +1,5 @@ import { LogContexts, LogLevels, Logger } from 'bs-logger' import memoize = require('lodash.memoize') -import micromatch = require('micromatch') import { basename, normalize, relative } from 'path' import * as _ts from 'typescript' @@ -8,7 +7,7 @@ import { ConfigSet } from '../config/config-set' import { CompilerInstance, MemoryCache, SourceOutput } from '../types' import { Errors, interpolate } from '../util/messages' -import { cacheResolvedModules, hasOwn } from './compiler-utils' +import { cacheResolvedModules, hasOwn, isTestFile } from './compiler-utils' function doTypeChecking(configs: ConfigSet, fileName: string, program: _ts.Program, logger: Logger) { if (configs.shouldReportDiagnostic(fileName)) { @@ -166,9 +165,8 @@ export const compileUsingProgram = (configs: ConfigSet, logger: Logger, memoryCa /** * We don't need the following logic with no cache run because no cache always gives correct typing */ - /* istanbul ignore next (covered by e2e) */ if (cacheDir) { - if (micromatch.isMatch(normalizedFileName, configs.testMatchPatterns)) { + if (isTestFile(configs.testMatchPatterns, normalizedFileName)) { cacheResolvedModules(normalizedFileName, code, memoryCache, program, cacheDir, logger) } else { /* istanbul ignore next (covered by e2e) */ diff --git a/src/config/config-set.spec.ts b/src/config/config-set.spec.ts index 8835b0a2cf..54f575787f 100644 --- a/src/config/config-set.spec.ts +++ b/src/config/config-set.spec.ts @@ -217,9 +217,9 @@ describe('tsJest', () => { expect(cs.tsJest.babelConfig).toBeUndefined() expect(cs.babel).toBeUndefined() expect(logger.target.lines[2]).toMatchInlineSnapshot(` -"[level:20] babel is disabled -" -`) + "[level:20] babel is disabled + " + `) }) it('should be correct for true', () => { @@ -294,9 +294,9 @@ describe('tsJest', () => { expect(cs.tsJest.babelConfig!.kind).toEqual('inline') expect(cs.babel).toEqual(expect.objectContaining(babelConfig)) expect(logger.target.lines[2]).toMatchInlineSnapshot(` -"[level:20] normalized babel config via ts-jest option -" -`) + "[level:20] normalized babel config via ts-jest option + " + `) }) it('should be correct for inline config', () => { @@ -312,9 +312,9 @@ describe('tsJest', () => { expect(cs.tsJest.babelConfig!.kind).toEqual('inline') expect(cs.babel).toEqual(expect.objectContaining(CONFIG)) expect(logger.target.lines[2]).toMatchInlineSnapshot(` -"[level:20] normalized babel config via ts-jest option -" -`) + "[level:20] normalized babel config via ts-jest option + " + `) }) }) // babelConfig @@ -425,15 +425,15 @@ describe('makeDiagnostic', () => { it('should set category', () => { expect(cs.makeDiagnostic(4321, 'foo might be bar', { category: ts.DiagnosticCategory.Error })) .toMatchInlineSnapshot(` -Object { - "category": 1, - "code": 4321, - "file": undefined, - "length": undefined, - "messageText": "foo might be bar", - "start": undefined, -} -`) + Object { + "category": 1, + "code": 4321, + "file": undefined, + "length": undefined, + "messageText": "foo might be bar", + "start": undefined, + } + `) }) }) // makeDiagnostic @@ -494,9 +494,9 @@ describe('typescript', () => { esModuleInterop: false, }) expect(target.lines.warn.join()).toMatchInlineSnapshot(` -"[level:40] message TS151001: If you have issues related to imports, you should consider setting \`esModuleInterop\` to \`true\` in your TypeScript configuration file (usually \`tsconfig.json\`). See https://blogs.msdn.microsoft.com/typescript/2018/01/31/announcing-typescript-2-7/#easier-ecmascript-module-interoperability for more information. -" -`) + "[level:40] message TS151001: If you have issues related to imports, you should consider setting \`esModuleInterop\` to \`true\` in your TypeScript configuration file (usually \`tsconfig.json\`). See https://blogs.msdn.microsoft.com/typescript/2018/01/31/announcing-typescript-2-7/#easier-ecmascript-module-interoperability for more information. + " + `) }) it('should not warn neither set synth. default imports if using babel', () => { @@ -976,11 +976,11 @@ describe('raiseDiagnostics', () => { expect(() => raiseDiagnostics([])).not.toThrow() expect(() => raiseDiagnostics([makeDiagnostic()])).not.toThrow() expect(logger.target.lines).toMatchInlineSnapshot(` - Array [ - "[level:40] [TS9999] foo - ", - ] - `) + Array [ + "[level:40] [TS9999] foo + ", + ] + `) }) }) diff --git a/src/config/config-set.ts b/src/config/config-set.ts index 3f79c61152..228efe4c56 100644 --- a/src/config/config-set.ts +++ b/src/config/config-set.ts @@ -186,6 +186,7 @@ export class ConfigSet { ...globals['ts-jest'], } } + this.logger.debug({ jestConfig: config }, 'normalized jest config') return config @@ -195,8 +196,8 @@ export class ConfigSet { * @internal */ @Memoize() - get testMatchPatterns(): string[] { - return this.jest.testMatch.concat(this.jest.testRegex) + get testMatchPatterns(): [string, RegExp] { + return this.jest.testMatch.concat(this.jest.testRegex) as [string, RegExp] } /** diff --git a/src/ts-jest-transformer.spec.ts b/src/ts-jest-transformer.spec.ts index 7c7e52a5a8..a02f43ce86 100644 --- a/src/ts-jest-transformer.spec.ts +++ b/src/ts-jest-transformer.spec.ts @@ -1,5 +1,4 @@ import { Config } from '@jest/types' -import stringify = require('fast-json-stable-stringify') import { sep } from 'path' import { ParsedCommandLine } from 'typescript' @@ -10,8 +9,7 @@ import { TsJestTransformer } from './ts-jest-transformer' describe('configFor', () => { it('should return the same config-set for same values with jest config string is not in configSetsIndex', () => { const obj1 = { cwd: '/foo/.', rootDir: '/bar//dummy/..', globals: {} } - const str = stringify(obj1) - const cs3 = new TsJestTransformer().configsFor(str) + const cs3 = new TsJestTransformer().configsFor(obj1 as any) expect(cs3.cwd).toBe(`${sep}foo`) expect(cs3.rootDir).toBe(`${sep}bar`) }) @@ -19,14 +17,11 @@ describe('configFor', () => { it('should return the same config-set for same values with jest config string in configSetsIndex', () => { const obj1 = { cwd: '/foo/.', rootDir: '/bar//dummy/..', globals: {} } const obj2 = { ...obj1 } - const str = stringify(obj1) const cs1 = new TsJestTransformer().configsFor(obj1 as any) const cs2 = new TsJestTransformer().configsFor(obj2 as any) - const cs3 = new TsJestTransformer().configsFor(str) expect(cs1.cwd).toBe(`${sep}foo`) expect(cs1.rootDir).toBe(`${sep}bar`) expect(cs2).toBe(cs1) - expect(cs3).toBe(cs1) }) }) @@ -242,17 +237,16 @@ describe('getCacheKey', () => { const tr = new TsJestTransformer() jest .spyOn(tr, 'configsFor') - .mockImplementation(jestConfigStr => (({ cacheKey: jestConfigStr } as unknown) as ConfigSet)) + .mockImplementation(jestConfigStr => (({ cacheKey: JSON.stringify(jestConfigStr) } as unknown) as ConfigSet)) const input = { fileContent: 'export default "foo"', fileName: 'foo.ts', jestConfigStr: '{"foo": "bar"}', - options: { instrument: false, rootDir: '/foo' }, + options: { config: { foo: 'bar' } as any, instrument: false, rootDir: '/foo' }, } const keys = [ tr.getCacheKey(input.fileContent, input.fileName, input.jestConfigStr, input.options), tr.getCacheKey(input.fileContent, 'bar.ts', input.jestConfigStr, input.options), - tr.getCacheKey(input.fileContent, input.fileName, '{}', input.options), tr.getCacheKey(input.fileContent, input.fileName, '{}', { ...input.options, instrument: true }), tr.getCacheKey(input.fileContent, input.fileName, '{}', { ...input.options, rootDir: '/bar' }), ] diff --git a/src/ts-jest-transformer.ts b/src/ts-jest-transformer.ts index d4e7514706..9618906003 100644 --- a/src/ts-jest-transformer.ts +++ b/src/ts-jest-transformer.ts @@ -1,11 +1,11 @@ -import { TransformOptions, TransformedSource, Transformer } from '@jest/transform/build/types' +import { CacheKeyOptions, TransformOptions, TransformedSource, Transformer } from '@jest/transform/build/types' import { Config } from '@jest/types' import { Logger } from 'bs-logger' import { inspect } from 'util' import { ConfigSet } from './config/config-set' import { TsJestGlobalOptions } from './types' -import { parse, stringify } from './util/json' +import { stringify } from './util/json' import { JsonableValue } from './util/jsonable-value' import { rootLogger } from './util/logger' import { Errors, interpolate } from './util/messages' @@ -58,28 +58,23 @@ export class TsJestTransformer implements Transformer { /** * Use by e2e, don't mark as internal */ - configsFor(jestConfig: Config.ProjectConfig | string): ConfigSet { - let csi: ConfigSetIndexItem | undefined - let jestConfigObj: Config.ProjectConfig - if (typeof jestConfig === 'string') { - csi = TsJestTransformer._configSetsIndex.find(cs => cs.jestConfig.serialized === jestConfig) - if (csi) return csi.configSet - jestConfigObj = parse(jestConfig) - } else { - csi = TsJestTransformer._configSetsIndex.find(cs => cs.jestConfig.value === jestConfig) - if (csi) return csi.configSet - // try to look-it up by stringified version - const serialized = stringify(jestConfig) - csi = TsJestTransformer._configSetsIndex.find(cs => cs.jestConfig.serialized === serialized) - if (csi) { - // update the object so that we can find it later - // this happens because jest first calls getCacheKey with stringified version of - // the config, and then it calls the transformer with the proper object - csi.jestConfig.value = jestConfig - return csi.configSet - } - jestConfigObj = jestConfig + configsFor(jestConfig: Config.ProjectConfig): ConfigSet { + let csi: ConfigSetIndexItem | undefined = TsJestTransformer._configSetsIndex.find( + cs => cs.jestConfig.value === jestConfig, + ) + if (csi) return csi.configSet + // try to look-it up by stringified version + const serialized = stringify(jestConfig) + csi = TsJestTransformer._configSetsIndex.find(cs => cs.jestConfig.serialized === serialized) + if (csi) { + // update the object so that we can find it later + // this happens because jest first calls getCacheKey with stringified version of + // the config, and then it calls the transformer with the proper object + csi.jestConfig.value = jestConfig + + return csi.configSet } + const jestConfigObj: Config.ProjectConfig = jestConfig // create the new record in the index this.logger.info(`no matching config-set found, creating a new one`) @@ -161,7 +156,7 @@ export class TsJestTransformer implements Transformer { * @see https://github.com/facebook/jest/blob/v23.5.0/packages/jest-runtime/src/script_transformer.js#L61-L90 * @param fileContent The content of the file * @param filePath The full path to the file - * @param jestConfigStr The JSON-encoded version of jest config + * @param _jestConfigStr The JSON-encoded version of jest config * @param transformOptions * @param transformOptions.instrument Whether the content will be instrumented by our transformer (always false) * @param transformOptions.rootDir Jest current rootDir @@ -169,12 +164,12 @@ export class TsJestTransformer implements Transformer { getCacheKey( fileContent: string, filePath: string, - jestConfigStr: string, - transformOptions: { instrument?: boolean; rootDir?: string } = {}, + _jestConfigStr: string, + transformOptions: CacheKeyOptions, ): string { this.logger.debug({ fileName: filePath, transformOptions }, 'computing cache key for', filePath) - const configs = this.configsFor(jestConfigStr) + const configs = this.configsFor(transformOptions.config) // we do not instrument, ensure it is false all the time const { instrument = false, rootDir = configs.rootDir } = transformOptions