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

refactor(jest-runtime): do not import from @jest/globals #12411

Closed

Conversation

mrazauskas
Copy link
Contributor

Summary

Currently jest-runtime is importing types from @jest/globals, seems like there is no need for that. All necessary types can be imported directly.

The only change – @jest/globals is removed from jest-runtime dependency graph.


Semi-related idea. If this change is accepted, none of the packages in the repo will depend on @jest/globals. What if the code of @jest/globals would be moved into jest? Just wondering, if the guide for typed testing could look like this:

  1. Install Jest: yarn add -D jest
  2. Import Jest: import {expect, jest, test} from 'jest';
  3. Happy typed testing!

Test plan

Green CI

@SimenB
Copy link
Member

SimenB commented Feb 16, 2022

A type test ensuring the types match would be nice, so we're sure they don't drift.

import type * as JestGlobalsPkg from '@jest/globals';
import type {JestGlobals} from 'jest-runtime';

expectAssignable<JestGlobalsPkg>({} as JestGlobals);
expectAssignable<JestGlobals>({} as JestGlobalsPkg);

What if the code of @jest/globals would be moved into jest?

Not sure how that would shake out for @types/jest - how confused would typescript be?

I still want @types/jest to augment the global env, so that part is opt-in

@mrazauskas
Copy link
Contributor Author

import type {JestGlobals} from 'jest-runtime';

Hm.. RuntimeGlobals and JestGlobals are not exported, but it might be good idea to export them. For example, would be useful here:

https://github.com/facebook/jest/blob/d4e22de47cbc2425bc52545afe2d348eaa5bba03/packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapterInit.ts#L37-L39

Another related idea is to move Jest interface from @jest/environment to jest-runtime. Feels like it belongs here. The @jest/globals dependency is the only obstacle. If Jest interface gets moved, then @jest/globals should import it from jest-runtime (perhaps to import JestGlobals from jest-runtime to @jest/globals and then there is no need for test?) Only that in this case @jest/globals will have huge dependency tree. Not sure if that’s good idea.

That’s why it seemed reasonable also to think about moving @jest/globals code to jest. In a way, importing into jest from jest-runtime does not add nothing to dependency tree.

Might be I am thinking about something very complicated, but the solution could be much more simple.

@SimenB
Copy link
Member

SimenB commented Feb 16, 2022

Or jest-runtime can import from @jest/globals 😀

@mrazauskas
Copy link
Contributor Author

And that is how it all work here. Got it (;

@mrazauskas mrazauskas closed this Feb 16, 2022
@mrazauskas mrazauskas deleted the refactor-do-not-import-globals branch February 16, 2022 20:25
@SimenB
Copy link
Member

SimenB commented Feb 16, 2022

I'm very interested in having import {jest} from 'jest' work, tho. So if we're sure it won't mess up @types/jest adding the types globally I think we can do it.

An alternative is a @jest/runtime package (confusing name with jest-runtime, but still) that is today's @jest/globals, and we change @jest/globals to augment the global environment. Nothing would import that by default when installing jest to leave global env alone. And jest could export @jest/runtime.

That way we wouldn't need @types/jest at all.

(as I typed out this alternative I got quite excited again, I think that can work out great long term (albeit some churn for people using @jest/globals today, but not too bad). I want Jest 29 to be veeeery light on breakage (I have plans for that to just be snapshot changes plus dropping node 17), but v28 can be super breaking)

@mrazauskas
Copy link
Contributor Author

Not sure I understood all details, but this might be brilliant. Breaking, but not too bad. I think, making it all work without @types/jest is ideal.

@github-actions
Copy link

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.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants