Skip to content

Add node 22, chrome 134, and ff138 images for base-internal and browsers-internal #1345

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

cacieprins
Copy link
Contributor

@cacieprins cacieprins commented May 13, 2025

This is in preparation for Electron 35.

Co-authored-by: Mike McCready <[email protected]>
README.md Outdated
Comment on lines 30 to 32
| [Google Chrome][chrome] | :white_check_mark: | see [#1188](https://github.com/cypress-io/cypress-docker-images/issues/1188) |
| [Mozilla Firefox][firefox] | :white_check_mark: | :white_check_mark: |
| [Microsoft Edge][edge] | :white_check_mark: | see [#1189](https://github.com/cypress-io/cypress-docker-images/issues/1189) |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't make these case changes in your internal PR.

The case is only used internally.

All the visible names are correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Prettier did this automatically - I'll revert

Copy link
Collaborator

Choose a reason for hiding this comment

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

@cacieprins

Let me check for Prettier separately! Thanks for explaining where it came from and for reverting.

package.json Outdated
@@ -33,5 +33,5 @@
"globals": "^16.0.0",
"markdown-link-check": "3.12.2",
"prettier": "3.5.3"
}
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
},
}

I missed removing the comma

Copy link
Collaborator

@MikeMcC399 MikeMcC399 left a comment

Choose a reason for hiding this comment

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

Thanks for the changes!

@MikeMcC399
Copy link
Collaborator

@cacieprins

I don't know what happened with Prettier. It's not asking for any format changes when I run it here.

$ npm ci

added 179 packages, and audited 180 packages in 3s

51 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
$ git switch node-22.14
$ npm run format:check

> [email protected] format:check
> prettier --check .

Checking formatting...
All matched files use Prettier code style!

CircleCI is happy as well:

https://app.circleci.com/pipelines/github/cypress-io/cypress-docker-images/2548/workflows/039ba3b7-0340-4082-87ab-aff2f29e4a97/jobs/78751

image

@MikeMcC399
Copy link
Collaborator

@cacieprins

For Corepack you can set COREPACK_ENABLE_AUTO_PIN=0 to stop it adding a packageManager field on its own.

I also added an enhancement request to ask Corepack NOT to change the package manager on its own, like here from npm to Yarn.

There are several other related enhancement requests for Corepack to change some of this and make it more user-friendly, however it looks like nobody is working on Corepack anymore since the Node.js TSC decided to stop distributing Corepack with Node.js 25 and above, so I don't know if it will ever get looked at.

@MikeMcC399
Copy link
Collaborator

@cacieprins

It looks like you must have had an old 2.x version of Prettier globally installed when you ran Prettier.

If you run npm ci before using npm run format then you should get the intended results using "prettier": "3.5.3".

There was a bug in Prettier 2.x where Markdown links were incorrectly changed to lowercase.


One of the Corepack maintainers has changed the auto-pin to be non-default. Hopefully that will get released soon and prevent accidental changes to package.json files!

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.

3 participants