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

fix(testing): puppeteer v10 support #2939

Merged
merged 4 commits into from Jun 25, 2021

Conversation

rwaskiewicz
Copy link
Member

@rwaskiewicz rwaskiewicz commented Jun 25, 2021

Important Front Matter

This PR is a port of #2934, which has largely already been reviewed:

The 'new stuff' includes:

  • d8ac36a - Lars' suggestions for merging declarations
  • A minor package-lock fix

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
    • Manual testing strategy described below
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build (npm run build) was run locally and any changes were pushed

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe): Backport of fix(testing): puppeteer v10 support #2934

What is the current behavior?

In this PR, we upgrade Stencil to allow support for v10.0.0 of Puppeteer. The version of Puppeteer (v5.5.0) supported in stencil@2.6.0 has two separate issues:

  1. npm install cannot be easily run in the Stencil repo itself on newer Mac machines (those using an M1). This was fixed on the Puppeteer side in feat(launcher): fix installation error on Apple M1 chips puppeteer/puppeteer#7099
  2. Existing clients cannot upgrade their version of Puppeteer

What is the new behavior?

  • Allows v10.0.0 of Puppeteer to be installed when developing for this repo and in downstream projects
  • Removes the usage of @types/puppeteer
    • In v6 of Puppeteer, the library moved from using DefinitelyTyped community sourced type declarations to generating/exporting their own
    • As a result, we must update Stencil code to conform to these new typings
    • Despite this, I don't believe this causes a breaking change, since our external API for testing is a facade in front of Puppeteer's
  • Updates internal typings to conform with Puppeteer

Does this introduce a breaking change?

  • Yes
  • No

Manual Testing

  • Ran npm build and npm pack from the root of the repo directory (on this branch)
  • Spun up a boilerplate Stencil app using the Stencil CLI npm init stencil
  • Installed the created tarball for my project
  • Installed the necessary test deps, and ran npm t in my stencil app passed
  • Dove 'into' the typings with both Puppeteer v5.5.0 and v10.0.0 to ensure there weren't any regressions with the type system/developer experience

Other information

  • I plan on squashing this branch when it's time to merge. I'll update the commit body at that time

This commit is a backport of #2934
- The intent is to allow folks developing for Stencil on the 2.X line
  with modern Mac machines to be able to run 'npm install' without
  resorting to workarounds regarding Puppeteer

Changes:
- Update puppeteer to v10.0.0
  - Remove @types/puppeteer from root package.json file
- Update programmatic library checks
  - Remove @types/puppeteer
  - Make puppeteer@10 the recommended version
- Replace @types/puppeteer instances of PageCloseOptions
  - This interface was originally defined in v5.4.3 of the DefinitelyTyped type declaration file for Puppeteer https://github.com/DefinitelyTyped/DefinitelyTyped/blob/34edf5fb8fdf54f57ed6584f77f1611767af7f6b/types/puppeteer/index.d.ts#L1423
  - It has since been removed, and replace with an object literal of the
    same shape
https://github.com/puppeteer/puppeteer/blob/main/src/common/Page.ts#L2097
- Replace usages of NavigationOptions for WaitForOptions
  - WaitForOptions is structurally the same as @types/puppeteer's
    [NavigationOptions](https://github.com/DefinitelyTyped/DefinitelyTyped/blob/34edf5fb8fdf54f57ed6584f77f1611767af7f6b/types/puppeteer/index.d.ts#L616)
and is used as a drop in replacement
  - The page#goTo method in Puppeteer's definition in both v5.4.3 and
    v10 allow an optional 'referer' string. Since it was omitted in the
    original implementation, I did not include it in this update
- Replace Response with HTTPResponse
- Update ElementHandle#press to use KeyInput instead of string
- Make typings of executionContext.evaluate explicit
- Migrate EmulateOptions
  - Puppeteer no longer exports a type of name EmulateOptions, in favor of
an object literal.
- Offer better compatability between Puppeteer 5 and 10 for projects
  that cannot upgrade
- TIL that lockfile V2 keeps track of caret/tildes in depedencies
@rwaskiewicz rwaskiewicz self-assigned this Jun 25, 2021
@rwaskiewicz rwaskiewicz requested a review from ltm June 25, 2021 17:59
@rwaskiewicz rwaskiewicz assigned ltm and unassigned rwaskiewicz Jun 25, 2021
@rwaskiewicz rwaskiewicz marked this pull request as ready for review June 25, 2021 17:59
@rwaskiewicz rwaskiewicz merged commit 09afd3f into master Jun 25, 2021
@rwaskiewicz rwaskiewicz deleted the rwaskiewicz/backport-puppeteer-fix branch June 25, 2021 18:35
@rwaskiewicz
Copy link
Member Author

This may resolve #2471, need to test further

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants