Remove explicit puppeteer dependency from package.json

[isabel_brison]

The explicit dependency was added here: ​https://github.com/WordPress/wordpress-develop/pull/347/files because puppeteer wasn't being properly installed as a dependency of the @wordpress/scripts package.

In the meantime, the version in package.json has fallen out of sync with @wordpress/scripts, so I tried removing it. After deleting node_modules and the lock file, and reinstalling everything, the tests are running without errors.

It's easier for maintainability if we don't have to update versions in two different places, so I propose removing puppeteer from package.json altogether.

Trac ticket: https://core.trac.wordpress.org/ticket/52843

[desrosj]

Thanks @isabel_brison! This makes sense to me!

Do you know when the dependency issue was fixed? This may be worth backporting to simplify the dependency list, but older branches don't all run the latest version of @wordpress/wp-scripts.

[desrosj]

In 50540:

Build/Test Tools: Remove explicit puppeteer dependency.

This was added in [48177] to fix an issue where Puppeteer was not being installed correctly as a dependency of @wordpress/wp-scripts. This has been fixed, so this explicit dependency can be removed.

Props isabel_brison.
Fixes #52843.

Merged into Core in https://core.trac.wordpress.org/changeset/50540. Just to note, there are a bunch of unrelated nested dependency updates here that I did not include.

[desrosj]

Looks like [50540] broke the E2E workflow. Investigating.

[desrosj]

I'm going to revert [50540] for the time being. I'm not sure why, but applying the diff from the PR verbatim locally is producing a Failed to load resource: the server responded with a status of 500 (Internal Server Error) error as well.

[desrosj]

In 50542:

Build/Test Tools: Revert [50540].

The E2E workflow is failing after this change. Reverting to to investigate further.

Unprops desrosj.
See #52843.

[isabel_brison]

@desroj, did you regenerate the the package-lock before committing? I notice that your commit just removes the root puppeteer dependency on line 18646, but the package-lock diff in my PR shows it instead to be replaced by the updated version: ​https://github.com/WordPress/wordpress-develop/pull/1102/files#diff-053150b640a7ce75eff69d1a22cae7f0f94ad64ce9a855db544dda0929316519L18646, and the nested dependency of wp-scripts being removed: ​https://github.com/WordPress/wordpress-develop/pull/1102/files#diff-053150b640a7ce75eff69d1a22cae7f0f94ad64ce9a855db544dda0929316519L3915

I only got it to work locally after deleting my node_modules and re-installing, so you may have to do that too. In any case, to avoid errors it's better to commit the whole package-lock changeset.

[desrosj]

@isabel_brison I tried deleting node_modules, and even erasing my package-lock.json file and regenerating on npm install, but I'm still seeing the test fail for me locally. Not sure what I'm missing.

[isabel_brison]

Hmmm, that's weird. Which version of node/npm are you on? Mine is node: 14.15.4 npm: 6.14.10. I've also tested on a Windows 10 PC with node: 12.16.1 and npm: 6.14.5 and it worked well after deleting node_modules :/

If you look into your node_modules do you see the puppeteer folder in the root directory?

[SergeyBiryukov]

Just noting that with node 12.17.0 and npm 6.14.5 I initially got the same package-lock.json file as in [50540] when following the steps from comment:10.

After running npm cache clean -f and updating to node 14.16.0 and npm 6.14.11, I got a ​slightly different package-lock.json file, and with that one the ​E2E workflow seems to pass.

Let's try this again and see if it works :)

[SergeyBiryukov]

In 50612:

Build/Test Tools: Remove explicit puppeteer dependency.

This was added in [48177] to fix an issue where Puppeteer was not being installed correctly as a dependency of @wordpress/wp-scripts. This has been fixed, so this explicit dependency can be removed.

Props isabel_brison, desrosj, SergeyBiryukov.
Fixes #52843.

[desrosj]

I'm still seeing some errors locally when running npm run test e2e:

> node ./tests/e2e/run-tests.js

Chromium is already in /Users/username/Sites/wordpress-develop/node_modules/@wordpress/scripts/node_modules/puppeteer/.local-chromium/mac-818858; skipping download.
Error: Cannot find module 'puppeteer'
Require stack:
- /Users/username/Sites/wordpress-develop/node_modules/jest-environment-puppeteer/lib/readConfig.js
- /Users/username/Sites/wordpress-develop/node_modules/jest-environment-puppeteer/lib/global.js
- /Users/username/Sites/wordpress-develop/node_modules/jest-environment-puppeteer/setup.js
- /Users/username/Sites/wordpress-develop/node_modules/@jest/transform/build/ScriptTransformer.js
- /Users/username/Sites/wordpress-develop/node_modules/@jest/transform/build/index.js
- /Users/username/Sites/wordpress-develop/node_modules/jest-runtime/build/index.js
- /Users/username/Sites/wordpress-develop/node_modules/@jest/core/build/cli/index.js
- /Users/username/Sites/wordpress-develop/node_modules/@jest/core/build/jest.js
- /Users/username/Sites/wordpress-develop/node_modules/jest/node_modules/jest-cli/build/cli/index.js
- /Users/username/Sites/wordpress-develop/node_modules/jest/node_modules/jest-cli/build/index.js
- /Users/username/Sites/wordpress-develop/node_modules/jest/build/jest.js
- /Users/username/Sites/wordpress-develop/node_modules/@wordpress/scripts/scripts/test-e2e.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:880:15)
    at Function.Module._load (internal/modules/cjs/loader.js:725:27)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at getPuppeteer (/Users/username/Sites/wordpress-develop/node_modules/jest-environment-puppeteer/lib/readConfig.js:58:14)
    at setup (/Users/username/Sites/wordpress-develop/node_modules/jest-environment-puppeteer/lib/global.js:21:50)
    at async /Users/username/Sites/wordpress-develop/node_modules/@jest/core/build/runGlobalHook.js:82:11
    at async waitForPromiseWithCleanup (/Users/username/Sites/wordpress-develop/node_modules/@jest/transform/build/ScriptTransformer.js:207:5)
    at async /Users/username/Sites/wordpress-develop/node_modules/@jest/core/build/runGlobalHook.js:72:9
    at async pEachSeries (/Users/username/Sites/wordpress-develop/node_modules/p-each-series/index.js:8:23)
child_process.js:655
    throw err;
    ^

Error: Command failed: wp-scripts test-e2e --config tests/e2e/jest.config.js
    at checkExecSyncError (child_process.js:616:11)
    at execSync (child_process.js:652:15)
    at Object.<anonymous> (/Users/username/Sites/wordpress-develop/tests/e2e/run-tests.js:9:1)
    at Module._compile (internal/modules/cjs/loader.js:1063:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:72:12)
    at internal/main/run_main_module.js:17:47 {
  status: 1,
  signal: null,
  output: [ null, null, null ],
  pid: 77038,
  stdout: null,
  stderr: null
}

When I run npm list puppeteer, I get the following output:

WordPress@5.8.0 /Users/username/Sites/wordpress-develop
├─┬ @wordpress/scripts@13.0.3
│ └── UNMET PEER DEPENDENCY puppeteer@npm:puppeteer-core@5.5.0
└─┬ grunt-contrib-qunit@4.0.0
  └── puppeteer@4.0.1

npm ERR! peer dep missing: puppeteer@>= 1.5.0 < 3, required by jest-puppeteer@4.4.0

I am on MacOS running Node 14.16.0/NPM 6.14.11. I removed my node_modules folder and ran npm install fresh and still get that issue.

[desrosj]

Doing some more debugging, it seems that my lock file is being updated to remove the dependency when running npm install. 52843.diff​ has the changes I'm seeing.

[isabel_brison]

@desrosj did you clear your npm cache before reinstalling?

So far I've only been able to reproduce your issue after symlinking my local @wordpress/scripts package to wordpress-develop. After doing that, deleting node_modules and reinstalling produced exactly the same diff as yours, but repeating the process installed correctly.

[isabel_brison]

@desrosj I'm wondering if you could have something in your user .npmrc that's stopping npm from flattening dependencies?

[desrosj]

More testing results:

• Ran npm cache clean --force.
• rm -rf node_modules
• npm install

Still see the issue and running npm list puppeteer still shows the unmet peer dependency.

I was playing around with v2 format of the lock file today for #52951, and oddly enough when the lock file is converted to the new v2 format, I don't have this problem. Maybe updating the file format is enough to resolve this?

[desrosj]

I'm not sure why, but I'm no longer seeing the problems above locally. Going to close this out.

Closing as this has been committed.