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

chore: Use peerDependencies marked as optional #675

Merged
merged 5 commits into from
Jul 7, 2022
Merged

chore: Use peerDependencies marked as optional #675

merged 5 commits into from
Jul 7, 2022

Conversation

berstend
Copy link
Owner

@berstend berstend commented Jul 7, 2022

Fixes: #527, #645, #672, #587

peerDependenciesMeta with optional: true is supported in yarn for a good while now and starting in npm v7

In addition our plugins have a transitive peer dependency on puppeteer-extra/playwright-extra through puppeteer-extra-plugin which apparently needs to be specified directly in each plugin for package managers to be happy

More context around this can be found in this comment: #527 (comment)

References:

@github-actions github-actions bot added the package: core Affecting a core package label Jul 7, 2022
@github-actions github-actions bot added plugin: puppeteer-extra PuppeteerExtra Plugin related plugin: recaptcha 🏴 reCAPTCHA plugin related plugin: stealth ㊙️ Detection evasion related labels Jul 7, 2022
@berstend berstend merged commit 3b72eaa into master Jul 7, 2022
@berstend berstend deleted the fix-527 branch July 7, 2022 18:58
@berstend
Copy link
Owner Author

berstend commented Jul 7, 2022

@rtritto yarn canary seems to be happy now 😄

yarn init -y
success Saved package.json
✨  Done in 0.06s.

yarn set version canary
➤ YN0000: Retrieving https://repo.yarnpkg.com/4.0.0-rc.11/packages/yarnpkg-cli/bin/yarn.js
➤ YN0000: Saving the new release in .yarn/releases/yarn-4.0.0-rc.11.cjs
➤ YN0000: Done in 1s 295ms

yarn add puppeteer [email protected] [email protected]
➤ YN0000: ┌ Resolution step
➤ YN0000: └ Completed in 4s 246ms
➤ YN0000: ┌ Fetch step
➤ YN0013: │ webidl-conversions@npm:3.0.1 can't be found in the cache and will be fetched from the remote registry
➤ YN0013: │ whatwg-url@npm:5.0.0 can't be found in the cache and will be fetched from the remote registry
➤ YN0013: │ wrappy@npm:1.0.2 can't be found in the cache and will be fetched from the remote registry
➤ YN0013: │ ws@npm:8.8.0 can't be found in the cache and will be fetched from the remote registry
➤ YN0013: │ yauzl@npm:2.10.0 can't be found in the cache and will be fetched from the remote registry
➤ YN0000: └ Completed in 4s 182ms
➤ YN0000: ┌ Link step
➤ YN0007: │ puppeteer@npm:15.3.1 must be built because it never has been before or the last one failed
➤ YN0000: └ Completed in 26s 186ms
➤ YN0000: Done in 34s 656ms

@rtritto
Copy link

rtritto commented Jul 7, 2022

@berstend thanks!

#644 still not fixed.

What do you think about my 2 reviews?

@berstend
Copy link
Owner Author

berstend commented Jul 7, 2022

What do you think about my 2 reviews?

In this PR? Can't see them 🤔

@rtritto
Copy link

rtritto commented Jul 7, 2022

Yes, you can check comments or changed files.

@berstend
Copy link
Owner Author

berstend commented Jul 7, 2022

@rtritto fixed in a82ab1c

Successfully published:
 - [email protected]
yarn add [email protected]                   
➤ YN0000: ┌ Resolution step
➤ YN0000: └ Completed in 8s 848ms
➤ YN0000: ┌ Fetch step
➤ YN0013: │ puppeteer-extra-plugin-adblocker@npm:2.13.2 can't be found in the cache and will be fetched from the remote registry
➤ YN0000: └ Completed in 0s 401ms
➤ YN0000: ┌ Link step
➤ YN0000: └ Completed
➤ YN0000: Done in 9s 339ms

@rtritto
Copy link

rtritto commented Jul 8, 2022

@berstend Reviews:

  • playwright-extra is used only with playwright, so I think that peerDependenciesMeta must be removed.

packages/playwright-extra/package.json:

  "peerDependencies": {
    "playwright": "*",
    "playwright-core": "*"
-  },
-  "peerDependenciesMeta": {
-    "playwright": {
-      "optional": true
-    },
-    "playwright-core": {
-      "optional": true
-    }
  }
  • @types/puppeteer is a devDependencies, must be removed from peerDependencies and peerDependenciesMeta.

packages/puppeteer-extra/package.json:

"peerDependencies": {
    "puppeteer": "*",
    "puppeteer-core": "*",
-    "@types/puppeteer": "*"
  },
  "peerDependenciesMeta": {
    "puppeteer": {
      "optional": true
    },
    "puppeteer-core": {
      "optional": true
-    },
-    "@types/puppeteer": {
-      "optional": true
-    }
  },

@berstend
Copy link
Owner Author

berstend commented Jul 8, 2022

@rtritto

  • playwright-extra can be used with playwright-core as well, if anything we should only have -core as the regular package is just a shim for that
  • @types/puppeteer is required when using an old puppeteer version which doesn't ship with types

is this a hypothetical discussion or are there some issues currently?

@rtritto
Copy link

rtritto commented Jul 8, 2022

@berstend thanks for answers.

is this a hypothetical discussion or are there some issues currently?

I don't know, for me there are no open points.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: core Affecting a core package plugin: puppeteer-extra PuppeteerExtra Plugin related plugin: recaptcha 🏴 reCAPTCHA plugin related plugin: stealth ㊙️ Detection evasion related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Use puppeteer-core as peer dependency instead of puppeteer
2 participants