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

feat(jest-config): Support using esbuild-register for loading TS configs #13742

Closed
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
### Features

- `[expect, @jest/expect-utils]` Support custom equality testers ([#13654](https://github.com/facebook/jest/pull/13654))
- `[jest-config]` Support using esbuild-register for loading TS configs ([#13742](https://github.com/facebook/jest/pull/13742))
- `[jest-config, jest-worker]` Use `os.availableParallelism` if available to calculate number of workers to spawn ([#13738](https://github.com/facebook/jest/pull/13738))
- `[@jest/globals, jest-mock]` Add `jest.replaceProperty()` that replaces property value ([#13496](https://github.com/facebook/jest/pull/13496))
- `[jest-haste-map]` ignore Sapling vcs directories (`.sl/`) ([#13674](https://github.com/facebook/jest/pull/13674))
Expand Down
2 changes: 1 addition & 1 deletion docs/Configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export default async (): Promise<Config> => {

:::tip

To read TypeScript configuration files Jest requires [`ts-node`](https://npmjs.com/package/ts-node). Make sure it is installed in your project.
To read TypeScript configuration files Jest by default requires [`ts-node`](https://npmjs.com/package/ts-node). You can override this behavior by adding a `@jest-config-loader` docblock at the top of the file. Currently, [`ts-node`](https://npmjs.com/package/ts-node) and [`esbuild-register`](https://npmjs.com/package/esbuild-register) is supported. Make sure `ts-node` or the loader you specify is installed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we have an example of a docblock? in case folks don't know what it is


:::

Expand Down
18 changes: 15 additions & 3 deletions e2e/__tests__/readInitialOptions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,21 @@ describe('readInitialOptions', () => {
expect(config).toEqual({jestConfig: 'package.json', rootDir});
expect(configPath).toEqual(configFile);
});
test('should read a jest.config.ts file', async () => {
SimenB marked this conversation as resolved.
Show resolved Hide resolved
const configFile = resolveFixture('ts-config', 'jest.config.ts');
const rootDir = resolveFixture('ts-config');
test('should read a jest.config.ts file with ts-node', async () => {
const configFile = resolveFixture('ts-node-config', 'jest.config.ts');
const rootDir = resolveFixture('ts-node-config');
const {config, configPath} = await proxyReadInitialOptions(undefined, {
cwd: rootDir,
});
expect(config).toEqual({jestConfig: 'jest.config.ts', rootDir});
expect(configPath).toEqual(configFile);
});
test('should read a jest.config.ts file with esbuild-register', async () => {
const configFile = resolveFixture(
'ts-esbuild-register-config',
'jest.config.ts',
);
const rootDir = resolveFixture('ts-esbuild-register-config');
const {config, configPath} = await proxyReadInitialOptions(undefined, {
cwd: rootDir,
});
Expand Down
11 changes: 11 additions & 0 deletions e2e/read-initial-options/ts-esbuild-register-config/jest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @jest-config-loader esbuild-register
*/
export default {
jestConfig: 'jest.config.ts',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a type annotation? Currently this file is valid JS

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, done in 66e69a1.

Copy link
Contributor Author

@MasterOdin MasterOdin Jan 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, not sure why using @jest/types worked locally and failed in CI, but replaced with a dummy interface in a42b463 which doesn't capture real-world usage as well, but does have TS specific syntax at least and doesn't require any sort of change to the CI.

Copy link
Contributor

@mrazauskas mrazauskas Jan 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can’t find the exact failure, but this note could be helpful. Only one job in CI builds types, all other tests run without types being build. So if you import some type from @jest/types, type check will fail.

Integration tests for ts-node used to read Jest config with type checks are ignored in jest.config.msj and run through jest.config.ts.mjs (with types build):

https://github.com/facebook/jest/blob/d2420aaf42055097dfe1e6a54bf3701214b06402/jest.config.mjs#L67
https://github.com/facebook/jest/blob/d2420aaf42055097dfe1e6a54bf3701214b06402/.github/workflows/nodejs.yml#L49-L50

Copy link
Contributor Author

@MasterOdin MasterOdin Jan 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, interesting.

Should I add testcases to e2e/__tests__/tsIntegration.test.ts for the new functionality? The testcases that check for a TS type failure won't fail the same way under esbuild-register since it doesn't do type/typescript checking, so would need to do different error checking.

For reference:

● using esbuild-register › when `Config` type is imported from "@jest/types" › throws if type errors are encountered

    expect(received).toMatch(expected)

    Expected substring: "jest.config.ts(3,40): error TS2322: Type 'string' is not assignable to type 'number'."
    Received string:    "● Validation Error:·
      Option \"testTimeout\" must be of type:
        number
      but instead received:
        string·
      Example:
      {
        \"testTimeout\": 5000
      }·
      Configuration Documentation:
      https://jestjs.io/docs/configuration
    "

      77 |       const {stderr, exitCode} = runJest(DIR);
      78 |
    > 79 |       expect(stderr).toMatch(
         |                      ^
      80 |         "jest.config.ts(3,40): error TS2322: Type 'string' is not assignable to type 'number'.",
      81 |       );
      82 |       expect(exitCode).toBe(1);

      at Object.toMatch (e2e/__tests__/tsIntegration.test.ts:79:22)

  ● using esbuild-register › when `Config` type is imported from "@jest/types" › throws if syntax errors are encountered

    expect(received).toMatch(expected)

    Expected substring: "jest.config.ts(4,16): error TS2304: Cannot find name 'get'."
    Received string:    "Error: Jest: Failed to parse the TypeScript config file /Users/mpeveler/code/github/jest/e2e/ts-node-integration/jest.config.ts
      Error: Transform failed with 1 error:
    /Users/mpeveler/code/github/jest/e2e/ts-node-integration/jest.config.ts:4:19: ERROR: Expected \";\" but found \"config\"
        at readConfigFileAndSetRootDir (/Users/mpeveler/code/github/jest/packages/jest-config/build/readConfigFileAndSetRootDir.js:123:13)
        at async readInitialOptions (/Users/mpeveler/code/github/jest/packages/jest-config/build/index.js:396:13)
        at async readConfig (/Users/mpeveler/code/github/jest/packages/jest-config/build/index.js:147:48)
        at async readConfigs (/Users/mpeveler/code/github/jest/packages/jest-config/build/index.js:417:26)
        at async runCLI (/Users/mpeveler/code/github/jest/packages/jest-core/build/cli/index.js:144:59)
        at async Object.run (/Users/mpeveler/code/github/jest/packages/jest-cli/build/run.js:124:37)"

       98 |       const {stderr, exitCode} = runJest(DIR);
       99 |
    > 100 |       expect(stderr).toMatch(
          |                      ^
      101 |         "jest.config.ts(4,16): error TS2304: Cannot find name 'get'.",
      102 |       );
      103 |       expect(exitCode).toBe(1);

      at Object.toMatch (e2e/__tests__/tsIntegration.test.ts:100:22)

  ● using esbuild-register › when `Config` type is imported from "@jest/types" › throws if type errors are encountered when package.json#type=module

    expect(received).toMatch(expected)

    Expected substring: "jest.config.ts(3,42): error TS2322: Type 'string' is not assignable to type 'number'."
    Received string:    "● Validation Error:·
      Option \"testTimeout\" must be of type:
        number
      but instead received:
        string·
      Example:
      {
        \"testTimeout\": 5000
      }·
      Configuration Documentation:
      https://jestjs.io/docs/configuration
    "

      159 |       const {stderr, exitCode} = runJest(DIR);
      160 |
    > 161 |       expect(stderr).toMatch(
          |                      ^
      162 |         "jest.config.ts(3,42): error TS2322: Type 'string' is not assignable to type 'number'.",
      163 |       );
      164 |       expect(exitCode).toBe(1);

      at Object.toMatch (e2e/__tests__/tsIntegration.test.ts:161:22)

  ● using esbuild-register › when `Config` type is imported from "@jest/types" › throws if syntax errors are encountered when package.json#type=module

    expect(received).toMatch(expected)

    Expected substring: "jest.config.ts(4,16): error TS2304: Cannot find name 'get'."
    Received string:    "Error: Jest: Failed to parse the TypeScript config file /Users/mpeveler/code/github/jest/e2e/ts-node-integration/jest.config.ts
      Error: Transform failed with 1 error:
    /Users/mpeveler/code/github/jest/e2e/ts-node-integration/jest.config.ts:4:21: ERROR: Expected \";\" but found \"config\"
        at readConfigFileAndSetRootDir (/Users/mpeveler/code/github/jest/packages/jest-config/build/readConfigFileAndSetRootDir.js:123:13)
        at async readInitialOptions (/Users/mpeveler/code/github/jest/packages/jest-config/build/index.js:396:13)
        at async readConfig (/Users/mpeveler/code/github/jest/packages/jest-config/build/index.js:147:48)
        at async readConfigs (/Users/mpeveler/code/github/jest/packages/jest-config/build/index.js:417:26)
        at async runCLI (/Users/mpeveler/code/github/jest/packages/jest-core/build/cli/index.js:144:59)
        at async Object.run (/Users/mpeveler/code/github/jest/packages/jest-cli/build/run.js:124:37)"

      180 |       const {stderr, exitCode} = runJest(DIR);
      181 |
    > 182 |       expect(stderr).toMatch(
          |                      ^
      183 |         "jest.config.ts(4,16): error TS2304: Cannot find name 'get'.",
      184 |       );
      185 |       expect(exitCode).toBe(1);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I add testcases to e2e/__tests__/tsIntegration.test.ts for the new functionality?

yes please 👍

};
7 changes: 7 additions & 0 deletions packages/jest-config/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,16 @@
},
"peerDependencies": {
"@types/node": "*",
"esbuild-register": ">=3.1.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we could resolve from the context if the config file and avoid the peer dep?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I follow what you mean here. My view was that this was documenting optional dependencies which can add additional functionality for jest-config, but I think could just totally remove it without it affecting the code at all.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taking a step back, we need the peer dep so that we can do import(moduleSpecified) and it resolves correctly (especially when using pnp or pnpm). However, if we resolve the module specified from the context of the config, we wouldn't need to specify the peer dep.

However, since I wanna change this in a future major so that we don't have to hard code support for modules in Jest, I think the current approach is fine for now 👍

"ts-node": ">=9.0.0"
},
"peerDependenciesMeta": {
"@types/node": {
"optional": true
},
"esbuild-register": {
"optional": true
},
"ts-node": {
"optional": true
}
Expand All @@ -39,6 +43,7 @@
"glob": "^7.1.3",
"graceful-fs": "^4.2.9",
"jest-circus": "workspace:^",
"jest-docblock": "workspace:^",
"jest-environment-node": "workspace:^",
"jest-get-type": "workspace:^",
"jest-regex-util": "workspace:^",
Expand All @@ -57,6 +62,8 @@
"@types/graceful-fs": "^4.1.3",
"@types/micromatch": "^4.0.1",
"@types/parse-json": "^4.0.0",
"esbuild": "^0.15.0",
"esbuild-register": "^3.1.0",
"semver": "^7.3.5",
"ts-node": "^10.5.0",
"typescript": "^4.8.2"
Expand Down
70 changes: 54 additions & 16 deletions packages/jest-config/src/readConfigFileAndSetRootDir.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,21 @@ import * as path from 'path';
import * as fs from 'graceful-fs';
import parseJson = require('parse-json');
import stripJsonComments = require('strip-json-comments');
import type {Service} from 'ts-node';
import type {Config} from '@jest/types';
import {extract, parse} from 'jest-docblock';
import {interopRequireDefault, requireOrImportModule} from 'jest-util';
import {
JEST_CONFIG_EXT_JSON,
JEST_CONFIG_EXT_TS,
PACKAGE_JSON,
} from './constants';

interface TsLoader {
enabled: (bool: boolean) => void;
}

type TsLoaderModule = 'ts-node' | 'esbuild-register';

// Read the configuration and set its `rootDir`
// 1. If it's a `package.json` file, we look into its "jest" property
// 2. If it's a `jest.config.ts` file, we use `ts-node` to transpile & require it
Expand Down Expand Up @@ -82,7 +88,19 @@ const loadTSConfigFile = async (
configPath: string,
): Promise<Config.InitialOptions> => {
// Get registered TypeScript compiler instance
const registeredCompiler = await getRegisteredCompiler();
const docblockPragmas = parse(extract(fs.readFileSync(configPath, 'utf8')));
const tsLoader = docblockPragmas['jest-config-loader'] || 'ts-node';
if (Array.isArray(tsLoader)) {
throw new Error(
`You can only define a single test environment through docblocks, got "${tsLoader.join(
', ',
)}"`,
);
}

const registeredCompiler = await getRegisteredCompiler(
tsLoader as TsLoaderModule,
);

registeredCompiler.enabled(true);

Expand All @@ -98,30 +116,50 @@ const loadTSConfigFile = async (
return configObject;
};

let registeredCompilerPromise: Promise<Service>;
let registeredCompilerPromise: Promise<TsLoader>;

function getRegisteredCompiler() {
function getRegisteredCompiler(loader: TsLoaderModule) {
// Cache the promise to avoid multiple registrations
registeredCompilerPromise = registeredCompilerPromise ?? registerTsNode();
registeredCompilerPromise =
registeredCompilerPromise ?? registerTsLoader(loader);
return registeredCompilerPromise;
}

async function registerTsNode(): Promise<Service> {
async function registerTsLoader(loader: TsLoaderModule): Promise<TsLoader> {
try {
// Register TypeScript compiler instance
const tsNode = await import('ts-node');
return tsNode.register({
compilerOptions: {
module: 'CommonJS',
},
moduleTypes: {
'**': 'cjs',
},
});
if (loader === 'ts-node') {
const tsLoader = await import('ts-node');
return tsLoader.register({
compilerOptions: {
module: 'CommonJS',
},
moduleTypes: {
'**': 'cjs',
},
});
} else if (loader === 'esbuild-register') {
const tsLoader = await import('esbuild-register/dist/node');
let instance: {unregister: () => void} | undefined;
return {
enabled: (bool: boolean) => {
if (bool) {
instance = tsLoader.register({
target: `node${process.version.slice(1)}`,
});
} else {
instance?.unregister();
}
},
};
}
throw new Error(
`Jest: '${loader}' is not a valid TypeScript configuration loader.`,
);
} catch (e: any) {
if (e.code === 'ERR_MODULE_NOT_FOUND') {
throw new Error(
`Jest: 'ts-node' is required for the TypeScript configuration files. Make sure it is installed\nError: ${e.message}`,
`Jest: '${loader}' is required for the TypeScript configuration files. Make sure it is installed\nError: ${e.message}`,
);
}

Expand Down
1 change: 1 addition & 0 deletions packages/jest-config/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
// jest-test-sequencer, but that is just `require.resolve`d, so no real use
// for their types
"references": [
{"path": "../jest-docblock"},
{"path": "../jest-environment-node"},
{"path": "../jest-get-type"},
{"path": "../jest-regex-util"},
Expand Down
Loading