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

Fixes #33560 - Pin @types/jest #8793

Merged
merged 1 commit into from Oct 19, 2021

Conversation

xprazak2
Copy link
Contributor

RTL jest-dom publishes its types in @types/testing-lib__jest-dom, which has open-ended dependency on @types/jest. That causes @types/jest 27.y.z to be pulled in, which in turns brings jest-diff and jest-get-type from the 27 release while other jest packages remain at version 26, which derails the tests.

@xprazak2 xprazak2 requested a review from a team as a code owner September 24, 2021 07:15
@theforeman-bot
Copy link
Member

Issues: #33560

@theforeman-bot
Copy link
Member

Issues: #33560

Copy link
Member

@evgeni evgeni left a comment

Choose a reason for hiding this comment

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

No problem packaging wise, we don't care about jest ;)

@@ -34,6 +34,7 @@
"@theforeman/stories": "^8.7.0",
"@theforeman/test": "^8.7.0",
"@theforeman/vendor-dev": "^8.7.0",
"@types/jest": "<27.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be @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.

I think I got this one right:
types-jest

Copy link
Member

Choose a reason for hiding this comment

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

huh, okay, the pr title was jest/types, so I thought that's correct ;)

@evgeni
Copy link
Member

evgeni commented Sep 24, 2021

Looking more closely, can you please add @jest to the excluded prefixes in https://github.com/theforeman/foreman/blob/develop/package-exclude.json please?

@theforeman-bot
Copy link
Member

@xprazak2, this pull request is currently not mergeable. Please rebase against the develop branch and push again.

If you have a remote called 'upstream' that points to this repository, you can do this by running:

    $ git pull --rebase upstream develop

This message was auto-generated by Foreman's prprocessor

@xprazak2
Copy link
Contributor Author

xprazak2 commented Oct 4, 2021

All green now.

@xprazak2
Copy link
Contributor Author

Can we get this pinned, pretty please?

ezr-ondrej
ezr-ondrej previously approved these changes Oct 18, 2021
Copy link
Member

@ezr-ondrej ezr-ondrej left a comment

Choose a reason for hiding this comment

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

Thanks @xprazak2 !

@ezr-ondrej
Copy link
Member

We need @evgeni to ack...

@@ -33,6 +33,7 @@
],
"EXCLUDE_NPM_PREFIXES": [
"@babel/eslint-",
"@jest/",
Copy link
Member

Choose a reason for hiding this comment

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

looks like the prefix below is @types, not @jest? :)

no idea if you want to exclude all @types here in prefixes, or just @types/jest in the packages list above -- I have no idea what @types is for a namespace

Copy link
Member

Choose a reason for hiding this comment

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

we definitelly don't want to exclude all @types, there are shared types accross packages, so eg patternfly can have types under that namespace.

@xprazak2 xprazak2 changed the title Fixes #33560 - Pin @jest/types Fixes #33560 - Pin @types/jest Oct 19, 2021
@xprazak2
Copy link
Contributor Author

Thanks, I added @types/jest to excluded packages.

Copy link
Member

@ezr-ondrej ezr-ondrej left a comment

Choose a reason for hiding this comment

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

Thanks @xprazak2 !

@ezr-ondrej ezr-ondrej merged commit eb4fc33 into theforeman:develop Oct 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants