Skip to content

Conversation

@mrazauskas
Copy link
Contributor

Summary

As it was noticed in #12856, test-utils could use build-in Jest types instead of @types/jest. Bumped into this again while working on another idea. Perhaps it makes sense to land this separately?

Test plan

Should build without type errors.

/* eslint-disable jest/no-focused-tests */

import semver = require('semver');
import type {Global} from '@jest/types';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just a reminder. It is not possible to import from @jest/globals, because @jest/globals depend on expect which depends on @jest/test-utils. That’s circular. In contrary @jest/types currently depends only on @jest/schemas.

Copy link
Member

Choose a reason for hiding this comment

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

We should fix the cycle - lerna warns every time I publish. Would be nice if yarn did the same on install

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Where does Lerna find a cycle?

In this comment I try to say that here importing from @jest/types does not create a cycle, but importing from @jest/globals would. In earlier PR it was your suggestion to import from @jest/globals. I tried that and hit the circular dips error. This is simply an explanation why I went for importing from @jest/types (;

Copy link
Member

Choose a reason for hiding this comment

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

pretty-format has a dev dep on expect

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. I recall that. It was added in #12626

Copy link
Member

Choose a reason for hiding this comment

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

Yup

* LICENSE file in the root directory of this source tree.
*/

/* eslint-disable jest/no-focused-tests */
Copy link
Member

Choose a reason for hiding this comment

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

this is weird. But I think it'd work if imported from @jest/globals, so probably fine 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. Just check, it works if imported from @jest/globals.

@SimenB SimenB merged commit d327483 into jestjs:main Sep 21, 2022
@mrazauskas mrazauskas deleted the feat-conditional-test-types branch September 21, 2022 08:00
@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 Oct 22, 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.

3 participants