Skip to content

Update WordPress Packages #347

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.

Sign up for GitHub

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

Closed
ellatrix wants to merge 8 commits into from
Closed

Update WordPress Packages #347

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ca1d287
Update packages
ellatrix Jun 25, 2020
4438950
Try nvm install --latest-npm
ellatrix Jun 25, 2020
21833cd
Fix qunit tests
youknowriad Jun 26, 2020
3d3e87f
Try to fix puppeteer
ellatrix Jun 26, 2020
5a02fa5
Fix php unit test
ellatrix Jun 26, 2020
2809bbf
Remove PUPPETEER_SKIP_CHROMIUM_DOWNLOAD flag
ellatrix Jun 26, 2020
e28c6bd
Fix typo
ellatrix Jun 26, 2020
a931153
Fix e2e test launcher
youknowriad Jun 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Try nvm install --latest-npm
  • Loading branch information
@ellatrix
ellatrix committed Jun 25, 2020
commit 443895096fdc510b596d02bfb862f8d9a5e0fd29
2 changes: 1 addition & 1 deletion .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ before_script:
fi
- npm --version
- node --version
- nvm install 10.13.0
- nvm install --latest-npm
Copy link

@ntsekouras ntsekouras Jun 26, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm installs nodejs and you removed it to update npm.

nvm install 10.13.0 I think it should stay as before (or use newer node.. I don't know the requirements ).

nvm install --latest-npm tries to update npm

Copy link
Member Author

@ellatrix ellatrix Jun 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, isn't this what Gutenberg does?

Copy link
Member Author

@ellatrix ellatrix Jun 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/WordPress/gutenberg/blob/4cc695898cedc8eb97b2bc2e4c63217cd17e6244/.travis.yml#L40

Copy link

@ntsekouras ntsekouras Jun 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm looking into Travis CI, as I haven't used it before, but it's definitely a different command, that does the above. Maybe node is already installed or installed in some other place.

Since it was there in WordPress Travis, it should stay there, except some change in node installation has happened.

Also the command --latest-npm is in different Travis hooks:
Wordpress: before_script
GB: before_install

Copy link

@ntsekouras ntsekouras Jun 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With nodejs installation comes the npm too.. So it might explain the Puppeteer not found module... ?

Copy link

@ntsekouras ntsekouras Jun 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot(1)

So it actually installs new nodejs

Copy link
Member

@gziolo gziolo Jun 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If Puppeteer isn’t installed then it means that the lock file wasn’t properly updated. The simplest way to fix it is to install the alias that wp-scripts uses as a dev dependency for the project, example:

https://github.com/WordPress/gutenberg/blob/7060b11b9b9efc00e099c9092fe520402d77ee4b/package.json#L180

It’s a bug in npm’s implementation.

Copy link
Member Author

@ellatrix ellatrix Jun 26, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're still getting this error:

Downloading Chromium r737027 - 118.4 Mb [====================] 100% 0.0s 
Chromium (737027) downloaded to /Applications/MAMP/htdocs/wordpress-git/node_modules/@wordpress/scripts/node_modules/puppeteer/.local-chromium/mac-737027
Error: Could not find browser revision 737027. Run "npm install" or "yarn install" to download a browser binary.
  at ChromeLauncher.launch (/Applications/MAMP/htdocs/wordpress-git/node_modules/puppeteer/lib/Launcher.js:200:23)

Copy link
Member Author

@ellatrix ellatrix Jun 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like Chromium is downloaded to node_modules/@wordpress/scripts/node_modules/puppeteer, while puppeteer runs from node_modules/puppeteer.

- |
if [[ "$NPM_INSTALL" == "true" ]]; then
npm ci
Expand Down