feat: add @jest/globals package for importing globals explicitly#9801
feat: add @jest/globals package for importing globals explicitly#9801SimenB merged 6 commits intojestjs:masterfrom
@jest/globals package for importing globals explicitly#9801Conversation
| export declare type xdescribe = Global.GlobalAdditions['xdescribe']; | ||
| export declare type fdescribe = Global.GlobalAdditions['fdescribe']; | ||
|
|
||
| throw new Error( |
There was a problem hiding this comment.
this file ensure types are correct, but also that it's not imported outside of jest. Compiled code is just the throw
|
|
||
| const FRAMEWORK_INITIALIZER = require.resolve('./jestAdapterInit'); | ||
| const FRAMEWORK_INITIALIZER = path.resolve(__dirname, './jestAdapterInit'); | ||
| const EXPECT_INITIALIZER = path.resolve(__dirname, './jestExpect.js'); |
There was a problem hiding this comment.
changes not related, just some local cleanup from looking into not adding the globals to the global env. Went with a simpler solution, but I liked these refactorings 🤷
| fs.existsSync(path.resolve(p, 'tsconfig.json')) | ||
| ); | ||
|
|
||
| packagesWithTs.forEach(pkgDir => { |
There was a problem hiding this comment.
moved to verify before compiling
jeysal
left a comment
There was a problem hiding this comment.
Nice workaround, although I kinda wish for a better world now where we inject things into people's test fns, and global APIs are accessed via gimmeSomeThing(__filename) etc. 😄
| export type TestName = Global.TestName; | ||
| export type TestFn = Global.TestFn; | ||
| export type HookFn = (done?: DoneFn) => Promise<any> | null | undefined; | ||
| export type HookFn = ( |
There was a problem hiding this comment.
wWhy the asymmetry? Promise<unknown> and null instead of Promise<any> like in Global.TestFn, Global.HookFn, and Circus.TestFn
There was a problem hiding this comment.
this was mostly me fixing a type error since it didn't accept void. Good point on the mismatch though - I'll push up exporting HookFn from Globals and reuse that here.
The type should ideally be Promise<void> | void, but then we don't get type errors if you return anything else. From my testing, void only matters to the caller, the callee doesn't have to fulfill that contract, it's just that the returned value cannot be used
There was a problem hiding this comment.
Hmm, looks like it does matter https://www.typescriptlang.org/play?#code/MYewdgzgLgBAHgLhgNxASwCYwLwwCwBMA3AFABmArmMFGuDGQBQCUSqmMA3iTLzAE4BTKBX5h8xEgF8gA
There was a problem hiding this comment.
Oh, nice! Can change then 👍 Should probably keep Promise<unknown> just so people can do test('t', () => promiseThing())
There was a problem hiding this comment.
Actually, nevermind, this might be the problem https://www.typescriptlang.org/play?#code/MYewdgzgLgBAZgLhgCgJQwLwD4YDcQCWAJpiutjACwBMA3EA
There was a problem hiding this comment.
ah, there you go! Adding |undefined makes it behave like we want, though
| throw new Error(`Package ${pkg.name} is missing \`types\` field`); | ||
| } | ||
|
|
||
| if (!pkg.typesVersions) { |
There was a problem hiding this comment.
Should this also check the content? Just to make sure they are all the same since they do all use the same downleveling build process.
There was a problem hiding this comment.
meh 😀 This is more "did you really mess up" than any thorough check
100% agreed, this is a first step 😀 |
| export type TestMode = BlockMode; | ||
| export type TestName = Global.TestName; | ||
| export type TestFn = Global.TestFn; | ||
| export type HookFn = (done?: DoneFn) => Promise<any> | null | undefined; |
There was a problem hiding this comment.
we don't actually accept null, we check for void 0 which is an non-strict mode way of saying undefined
|
Is it now possible to use Jest with Flow without having to install the flow-types definitions? |
|
No, we don't ship Flow definitions, only TypeScript. |
|
Bit of a shame those are not exported from import/no-extraneous-dependencies would force a |
|
They'll always get updated together, so shouldn't get out of sync in practice. We haven't changed our global API in years, so I doubt they'll drift anytime soon 🙂 |
|
@silverwind main reason for not implementing it in the Those imports are pretty useless, so one thing we could do is make |
|
One limitation I thought of now is that |
|
True, should be easy to fix in Babel plugin jest hoist though |
|
I tried (and failed) in #9806, help appreciated 🙂 |
|
Need to make a release, so I'll be reverting this until we're able to fix the Babel plugin (#9806). Will hopefully be included in the next release! |
…ls explicitly (jestjs#9801)"" This reverts commit 75ab1ab.
|
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. |
Summary
People have asked for it, so why not. We still add to the global env - we can remove that later if we want to (via some config flag I guess). If we want to do that it should be pretty straight forward, just add a
runtime.registerGlobalsfunction or some such and get the globals from that rather thanenvironment.global. I don't want it to beimport {test} from 'jest'as that's difficult to type correctly - a separate package can both ensure types are correct and that it's only used in a Jest environment.Fixes #4473
Fixes #5192
Closes #7571
Closes #9306
Test plan
Integration test added