Update jest-environment-puppeteer

[ifiokjr]

Please fill in this template.

• Use a meaningful title for the pull request. Include the name of the package modified.
• Test the change in your own code. (Compile and run.)
• Add or edit tests to reflect the change. (Run with npm test.)
• Follow the advice from the readme.
• Avoid common mistakes.
• Run npm run lint package-name (or tsc if no tslint.json is present).

If changing an existing definition:

• Provide a URL to documentation or source code which provides context for the suggested changes: <>
• If this PR brings the type definitions up to date with a new version of the JS library, update the version number in the header.
• Include tests for your changes
• If you are making substantial changes, consider adding a tslint.json containing { "extends": "dtslint/dt.json" }. If for reason the any rule need to be disabled, disable it for that line using // tslint:disable-next-line [ruleName] and not for whole package so that the need for disabling can be reviewed.

[typescript-bot]

@ifiokjr Thank you for submitting this PR!

This PR doesn't modify any tests, so it's hard to know what's being fixed, and your changes might regress in the future. Have you considered adding tests to cover the change you're making? Including tests allows this PR to be merged by yourself and the owners of this module. This can potentially save days of time for you.

Code Reviews

This PR can be merged once it's reviewed by a DT maintainer.

Status

• ✅ No merge conflicts
• ❌ Continuous integration tests have passed
• ❌ Only a DT maintainer can merge changes without tests

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.

---

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "now": "-",
  "pr_number": 44613,
  "author": "ifiokjr",
  "owners": [
    "joshuakgoldberg",
    "ifiokjr",
    "favna"
  ],
  "dangerLevel": "ScopedAndUntested",
  "headCommitAbbrOid": "f9022c0",
  "headCommitOid": "f9022c0aaca3c0897b5ee75f8502eb0060f332a7",
  "mergeIsRequested": false,
  "stalenessInDays": 8,
  "lastCommitDate": "2020-05-10T05:36:29.000Z",
  "lastCommentDate": "2020-05-10T11:03:12.000Z",
  "reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/44613/files",
  "hasMergeConflict": false,
  "authorIsOwner": true,
  "isFirstContribution": false,
  "popularityLevel": "Well-liked by everyone",
  "anyPackageIsNew": false,
  "packages": [
    "jest-environment-puppeteer"
  ],
  "files": [
    {
      "filePath": "types/jest-environment-puppeteer/index.d.ts",
      "kind": "definition",
      "package": "jest-environment-puppeteer"
    }
  ],
  "hasDismissedReview": false,
  "travisResult": "fail",
  "travisUrl": "https://travis-ci.org/github/DefinitelyTyped/DefinitelyTyped/builds/685234638?utm_source=github_status&utm_medium=notification",
  "reviewersWithStaleReviews": [],
  "approvalFlags": 0,
  "isChangesRequested": false
}

[typescript-bot]

@ifiokjr Thank you for submitting this PR!

This PR doesn't modify any tests, so it's hard to know what's being fixed, and your changes might regress in the future. Have you considered adding tests to cover the change you're making? Including tests allows this PR to be merged by yourself and the owners of this module. This can potentially save days of time for you.

Code Reviews

This PR can be merged once it's reviewed by a DT maintainer.

Status

• ✅ No merge conflicts
• ❌ Continuous integration tests have passed
• ❌ Only a DT maintainer can merge changes without tests

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.

---

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "now": "-",
  "pr_number": 44613,
  "author": "ifiokjr",
  "owners": [
    "joshuakgoldberg",
    "ifiokjr",
    "favna"
  ],
  "dangerLevel": "ScopedAndUntested",
  "headCommitAbbrOid": "f9022c0",
  "headCommitOid": "f9022c0aaca3c0897b5ee75f8502eb0060f332a7",
  "mergeIsRequested": false,
  "stalenessInDays": 0,
  "lastCommitDate": "2020-05-10T05:36:29.000Z",
  "lastCommentDate": "2020-05-10T05:36:29.000Z",
  "reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/44613/files",
  "hasMergeConflict": false,
  "authorIsOwner": true,
  "isFirstContribution": false,
  "popularityLevel": "Well-liked by everyone",
  "anyPackageIsNew": false,
  "packages": [
    "jest-environment-puppeteer"
  ],
  "files": [
    {
      "filePath": "types/jest-environment-puppeteer/index.d.ts",
      "kind": "definition",
      "package": "jest-environment-puppeteer"
    }
  ],
  "hasDismissedReview": false,
  "travisResult": "missing",
  "reviewersWithStaleReviews": [],
  "approvalFlags": 0,
  "isChangesRequested": false
}

[typescript-bot]

🔔 @JoshuaKGoldberg @favna - 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]

🔔 @JoshuaKGoldberg @favna - 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]

🔔 @JoshuaKGoldberg @favna - 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.

[favna]

CI is failing due to know issue, see comments on #44033 and possibly also #44365

I recommend you change this to a draft so the bot doesn't close it and come back to it once the issues are resolved.

Also as per #44033 - I'm not sure if this would even fix the issue the package has currently. Removing one property isn't going to make TS not complain it's missing another, it's going to make it complain it's missing both. Unless the class signature is overloaded anyway.

[ifiokjr]

CI is failing due to know issue, see comments on #44033 and possibly also #44365

Thanks good to know 👍

I recommend you change this to a draft so the bot doesn't close it and come back to it once the issues are resolved.

I don't seem to be able to.

Also as per #44033 - I'm not sure if this would even fix the issue the package has currently. Removing one property isn't going to make TS not complain it's missing another, it's going to make it complain it's missing both. Unless the class signature is overloaded anyway.

I've been using the fix for a while now via patch package, but only just getting around to adding it here.

https://github.com/remirror/remirror/blob/c34f86e0e7ff8fd054196804d3cdf7055e4c7f61/support/patches/%40types%2Bjest-environment-puppeteer%2B4.3.1.patch#L1-L15

I don't even remember what the issue was anymore, but the type check is passing in my codebase without skipLibCheck.

[favna]

I've been using the fix for a while now via patch package, but only just getting around to adding it here.
remirror/remirror:support/patches/%40types%2Bjest-environment-puppeteer%2B4.3.1.patch@c34f86e#L1-L15
I don't even remember what the issue was anymore, but the type check is passing in my codebase without skipLibCheck.

Ah that sounds good then, the problem in #44033 was that TS was complaining about missing fakeTimersLolex. I can't really dive into it but I'm going to do a shot in the dark and say that Jest might have an overloaded class, and so when fakeTimers property is present it goes to the overload where it also needs fakeTimersLolex, but when the property is not there (like with your version) then it goes to a version of the class without fakeTimersLolex.

I recommend you change this to a draft so the bot doesn't close it and come back to it once the issues are resolved.

I don't seem to be able to.

It should be right under the reviewers section to the right of your PR? Screenshot taken from another open PR of mine elsewhere. See right under the user "gc".

[ifiokjr]

@favna thanks for the help.

Has the issue now been fixed and am I okay to push this up again?

[favna]

Well.. there has since been a new release of Jest (they release v26 after the PR was merged that fixed the issue).. so give it a shot I guess and see if the CI passed. Note that you can also run the same tests as Travis does locally by running npm run test. Be sure to prune your node_modules and reinstall them once before you do - as well as rebase on top of master so you can be sure you have latest versions of everything.

So basically:

# For Linux / MacOS / WSL 
rm -rf node_modules
# For Windows Powershell
Remove-Item -Recurse -Force node_modules

# All:
git remote add upstream https://github.com/DefinitelyTyped/DefinitelyTyped
git fetch --all --prune
git rebase upstream/master
# <finish the rebase process, there should be no conflicts afaik>
npm install
npm run test

Whether it'll all work out I can't tell - but I sure hope it does.

[peterblazejewicz]

To be reviewed soon, if should be closed, until some action takes place