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

[jest] constrain type of the jest.replacedProperty() argument #64936

Merged
merged 4 commits into from Apr 19, 2023
Merged

[jest] constrain type of the jest.replacedProperty() argument #64936

merged 4 commits into from Apr 19, 2023

Conversation

mrazauskas
Copy link
Contributor

Please fill in this template.

If changing an existing definition:

Summary

Based on user report (see link above). Currently if compiler option "skipLibCheck": false is set and @types/jest are used with Jest’s built-in types, the compiler gives following error:

node_modules/@jest/environment/build/index.d.ts:401:26 - error TS2430: Interface 'JestImportMeta' incorrectly extends interface 'ImportMeta'.
  The types of 'jest.replaceProperty' are incompatible between these types.
    Type '<T extends object, K extends Exclude<keyof T, keyof { [K in keyof T as Required<T>[K] extends ClassLike ? K : never]: T[K]; } | keyof { [K in keyof T as Required<T>[K] extends FunctionLike ? K : never]: T[K]; }>, V extends T[K]>(object: T, propertyKey: K, value: V) => Replaced<...>' is not assignable to type '<T, K extends keyof T>(obj: T, key: K, value: T[K]) => ReplaceProperty<T[K]>'.
      Types of parameters 'object' and 'obj' are incompatible.
        Type 'T' is not assignable to type 'object'.

401 export declare interface JestImportMeta extends ImportMeta {
                             ~~~~~~~~~~~~~~

  node_modules/@types/jest/index.d.ts:331:30
    331     function replaceProperty<T, K extends keyof T>(obj: T, key: K, value: T[K]): ReplaceProperty<T[K]>;
                                     ~
    This type parameter might need an `extends object` constraint.

This is because the jest.replacedProperty() argument is missing a constrain in @types/jest. jest.spyOn() is similar, so I added the same {} constrain (note that in Jest built-in types the constrain is object):

function spyOn<T extends {}, M extends ConstructorPropertyNames<Required<T>>>(

@typescript-bot
Copy link
Contributor

typescript-bot commented Mar 29, 2023

@mrazauskas Thank you for submitting this PR!

This is a live comment which I will keep updated.

1 package in this PR

Code Reviews

Because you edited one package and updated the tests (👏), I can help you merge this PR once someone else signs off on it.

You can test the changes of this PR in the Playground.

Status

  • ✅ No merge conflicts
  • ✅ Continuous integration tests have passed
  • ✅ Most recent commit is approved by type definition owners or DT maintainers

All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes.


Diagnostic Information: What the bot saw about this PR
{
  "type": "info",
  "now": "-",
  "pr_number": 64936,
  "author": "mrazauskas",
  "headCommitOid": "2eed5aa3bb89cdb5e96a1f92f5d758f830eb2ef3",
  "mergeBaseOid": "498a65e3da2f685fc7b86e99f4094bddf1512459",
  "lastPushDate": "2023-04-18T18:38:58.000Z",
  "lastActivityDate": "2023-04-19T09:41:05.000Z",
  "maintainerBlessed": "Waiting for Code Reviews",
  "mergeOfferDate": "2023-04-19T09:40:27.000Z",
  "mergeRequestDate": "2023-04-19T09:41:05.000Z",
  "mergeRequestUser": "mrazauskas",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "jest",
      "kind": "edit",
      "files": [
        {
          "path": "types/jest/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/jest/jest-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "NoHomey",
        "jwbay",
        "asvetliakov",
        "alexjoverm",
        "epicallan",
        "ikatyang",
        "wsmd",
        "JamieMason",
        "douglasduteil",
        "ahnpnl",
        "UselessPickles",
        "r3nya",
        "hotell",
        "sebald",
        "andys8",
        "antoinebrault",
        "gstamac",
        "ExE-Boss",
        "quassnoi",
        "Belco90",
        "tonyhallett",
        "ycmjason",
        "pawfa",
        "gerkindev",
        "domdomegg",
        "mrazauskas"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "domdomegg",
      "date": "2023-04-19T09:39:40.000Z",
      "isMaintainer": false
    }
  ],
  "mainBotCommentID": 1488891670,
  "ciResult": "pass"
}

@typescript-bot typescript-bot added Where is GH Actions? GH Actions didn't give a response to this PR Critical package Author is Owner The author of this PR is a listed owner of the package. labels Mar 29, 2023
@typescript-bot
Copy link
Contributor

🔔 @NoHomey @jwbay @asvetliakov @alexjoverm @epicallan @ikatyang @wsmd @JamieMason @douglasduteil @ahnpnl @UselessPickles @r3nya @Hotell @sebald @andys8 @antoinebrault @gstamac @ExE-Boss @quassnoi @Belco90 @tonyhallett @ycmjason @pawfa @GerkinDev @domdomegg — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

@typescript-bot typescript-bot removed the Where is GH Actions? GH Actions didn't give a response to this PR label Apr 5, 2023
@typescript-bot typescript-bot moved this from Other to Waiting for Code Reviews in New Pull Request Status Board Apr 5, 2023
@typescript-bot typescript-bot moved this from Waiting for Code Reviews to Needs Maintainer Review in New Pull Request Status Board Apr 5, 2023
@sheetalkamat sheetalkamat moved this from Needs Maintainer Review to Waiting for Code Reviews in New Pull Request Status Board Apr 5, 2023
@typescript-bot
Copy link
Contributor

Re-ping @NoHomey, @jwbay, @asvetliakov, @alexjoverm, @epicallan, @ikatyang, @wsmd, @JamieMason, @douglasduteil, @ahnpnl, @UselessPickles, @r3nya, @Hotell, @sebald, @andys8, @antoinebrault, @gstamac, @ExE-Boss, @quassnoi, @Belco90, @tonyhallett, @ycmjason, @pawfa, @GerkinDev, @domdomegg:

This PR has been out for over a week, yet I haven't seen any reviews.

Could someone please give it some attention? Thanks!

@typescript-bot typescript-bot added the Unreviewed No one showed up to review this PR, so it'll be reviewed by a DT maintainer. label Apr 18, 2023
types/jest/index.d.ts Outdated Show resolved Hide resolved
Co-authored-by: Adam Jones <domdomegg+git@gmail.com>
@typescript-bot typescript-bot removed the Unreviewed No one showed up to review this PR, so it'll be reviewed by a DT maintainer. label Apr 18, 2023
@typescript-bot typescript-bot moved this from Waiting for Code Reviews to Needs Maintainer Review in New Pull Request Status Board Apr 18, 2023
@typescript-bot
Copy link
Contributor

@domdomegg Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

Co-authored-by: Adam Jones <domdomegg+git@gmail.com>
@typescript-bot typescript-bot moved this from Needs Maintainer Review to Waiting for Code Reviews in New Pull Request Status Board Apr 18, 2023
@typescript-bot typescript-bot moved this from Waiting for Code Reviews to Needs Maintainer Review in New Pull Request Status Board Apr 18, 2023
@RyanCavanaugh RyanCavanaugh moved this from Needs Maintainer Review to Waiting for Code Reviews in New Pull Request Status Board Apr 18, 2023
@typescript-bot typescript-bot added Owner Approved A listed owner of this package signed off on the pull request. Self Merge This PR can now be self-merged by the PR author or an owner labels Apr 19, 2023
@typescript-bot typescript-bot moved this from Waiting for Code Reviews to Waiting for Author to Merge in New Pull Request Status Board Apr 19, 2023
@mrazauskas
Copy link
Contributor Author

Ready to merge

@typescript-bot typescript-bot moved this from Waiting for Author to Merge to Recently Merged in New Pull Request Status Board Apr 19, 2023
@typescript-bot typescript-bot merged commit 683f1d9 into DefinitelyTyped:master Apr 19, 2023
2 checks passed
@domdomegg
Copy link
Contributor

domdomegg commented Apr 19, 2023

Thanks for the PR @mrazauskas 🙌 , and apologies for the delayed review on my part.

@molily
Copy link

molily commented Apr 19, 2023

Thanks to everyone who contributed to this! ❤️

@mrazauskas mrazauskas deleted the jest-constrain-replacedProperty-argument branch April 19, 2023 12:35
@typescript-bot typescript-bot removed this from Recently Merged in New Pull Request Status Board Apr 21, 2023
@xt0rted
Copy link
Contributor

xt0rted commented May 2, 2023

I have a PR updating from v29.4.0 to v29.5.1 of these types and I'm getting a similar build error as seen here xt0rted/dotnet-sdk-updater#391. Should this PR of fixed that error or is it a new one?

Error: node_modules/@jest/environment/build/index.d.ts(401,26): error TS2430: Interface 'JestImportMeta' incorrectly extends interface 'ImportMeta'.
  The types of 'jest.replaceProperty' are incompatible between these types.
    Type '<T extends object, K extends Exclude<keyof T, keyof { [K in keyof T as Required<T>[K] extends ClassLike ? K : never]: T[K]; } | keyof { [K in keyof T as Required<T>[K] extends FunctionLike ? K : never]: T[K]; }>, V extends T[K]>(object: T, propertyKey: K, value: V) => Replaced<...>' is not assignable to type '<T extends object, K extends keyof T>(obj: T, key: K, value: T[K]) => ReplaceProperty<T[K]>'.
      Types of parameters 'propertyKey' and 'key' are incompatible.
        Type 'K' is not assignable to type 'Exclude<keyof T, keyof { [K in keyof T as Required<T>[K] extends ClassLike ? K : never]: T[K]; } | keyof { [K in keyof T as Required<T>[K] extends FunctionLike ? K : never]: T[K]; }>'.
          Type 'keyof T' is not assignable to type 'Exclude<keyof T, keyof { [K in keyof T as Required<T>[K] extends ClassLike ? K : never]: T[K]; } | keyof { [K in keyof T as Required<T>[K] extends FunctionLike ? K : never]: T[K]; }>'.
            Type 'string | number | symbol' is not assignable to type 'Exclude<keyof T, keyof { [K in keyof T as Required<T>[K] extends ClassLike ? K : never]: T[K]; } | keyof { [K in keyof T as Required<T>[K] extends FunctionLike ? K : never]: T[K]; }>'.
              Type 'string' is not assignable to type 'Exclude<keyof T, keyof { [K in keyof T as Required<T>[K] extends ClassLike ? K : never]: T[K]; } | keyof { [K in keyof T as Required<T>[K] extends FunctionLike ? K : never]: T[K]; }>'.

@mrazauskas
Copy link
Contributor Author

The problem is on Jest side. It is fixed already, just not yet released (see jestjs/jest#14097).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author is Owner The author of this PR is a listed owner of the package. Critical package Owner Approved A listed owner of this package signed off on the pull request. Self Merge This PR can now be self-merged by the PR author or an owner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants