-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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 type ellision of jest-runtime imports #9717
Conversation
Hi @jimmydief! Thank you for your pull request and welcome to our community.We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
Hey, thanks for the PR! Unfortunately, this isn't the correct solution - it's a "bug" in the babel typescript transform where it isn't removed even though it's only used as a type. This is the correct fix: diff --git i/packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapter.ts w/packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapter.ts
index 34d214f38..2a9561d3a 100644
--- i/packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapter.ts
+++ w/packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapter.ts
@@ -9,7 +9,7 @@ import * as path from 'path';
import type {Config} from '@jest/types';
import type {JestEnvironment} from '@jest/environment';
import type {TestResult} from '@jest/test-result';
-import Runtime = require('jest-runtime');
+import type {RuntimeType as Runtime} from 'jest-runtime';
import type {SnapshotStateType} from 'jest-snapshot';
const FRAMEWORK_INITIALIZER = require.resolve('./jestAdapterInit');
diff --git i/packages/jest-runtime/src/index.ts w/packages/jest-runtime/src/index.ts
index 11bb29aa2..7c1390ebc 100644
--- i/packages/jest-runtime/src/index.ts
+++ w/packages/jest-runtime/src/index.ts
@@ -64,6 +64,8 @@ type CacheFS = {[path: string]: string};
namespace Runtime {
export type Context = JestContext;
+ // ditch this export when moving to esm - for now we need it for to avoid faulty type elision
+ export type RuntimeType = Runtime;
}
const testTimeoutSymbol = Symbol.for('TEST_TIMEOUT_SYMBOL'); Wanna update your PR to have this instead? EDIT: actually, your change is probably also needed, or the typings file is wrong. But it's not required at runtime, so the diff above should get in as well 🙂 EDIT2: Same issue in jasmine actually, but there it's already declared in diff --git i/packages/jest-jasmine2/src/index.ts w/packages/jest-jasmine2/src/index.ts
index d87e3451f..f8ab81713 100644
--- i/packages/jest-jasmine2/src/index.ts
+++ w/packages/jest-jasmine2/src/index.ts
@@ -10,7 +10,7 @@ import type {Config, Global} from '@jest/types';
import type {AssertionResult, TestResult} from '@jest/test-result';
import type {JestEnvironment} from '@jest/environment';
import type {SnapshotStateType} from 'jest-snapshot';
-import Runtime = require('jest-runtime');
+import type {RuntimeType as Runtime} from 'jest-runtime';
import {getCallsite} from '@jest/source-map';
import installEach from './each'; |
Thanks, makes sense! |
Thanks! |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Fixes #9600.
Summary
After refreshing my project's lockfile, I noticed an issue when running Jest with
jest-circus
as the runner due to a missingjest-runtime
dependency. Making it a non-dev dependency should fix this.Test plan
Installing the dependency at the top level fixes the issue in my project,
npm
seems to install it in a nested node_modules folder otherwise.