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: Don't hoist devtools-protocol #28372

Merged
merged 5 commits into from
Nov 22, 2023

Conversation

chrisbreiding
Copy link
Contributor

@chrisbreiding chrisbreiding commented Nov 21, 2023

Additional details

This is a prerequisite for #28370.puppeteer-core has a dependency on a version of devtools-protocol which is incompatible with the version we use in various packages in the repo. If they get hoisted, it uses the version installed by puppeteer-core and causes type mismatches and CI failures.

This needs to be separate from #28370 since otherwise the release analyzer thinks it's making changes to the binary that requires a changelog entry, but #28370 is only a feature release for the @cypress/puppeteer npm package.

PR Tasks

Copy link

cypress bot commented Nov 21, 2023

@emilyrohrbough
Copy link
Member

@chrisbreiding what does one need to know with updating this in the future? are we ahead or behind playwright? Any reason we couldn't match versions?

@chrisbreiding
Copy link
Contributor Author

@emilyrohrbough I don't think there's much to be concerned about in the future. If devtools-protocol is added as a dependency of another package and it's causing type-check errors, then it would also need to nohoist it.

As far as matching versions, we could certainly do that. I tried that out, but it would require changing some of the devtools-protocol types we reference and I didn't want to open that can of worms for the time being.

@emilyrohrbough
Copy link
Member

The concern would really just be forgetting why it is here and it being harder to unwind and remove with all the other dependencies in the repo.

@chrisbreiding
Copy link
Contributor Author

Yeah, I understand that concern. If only package.json supported comments. At the very least, the git blame can lead someone in the future back to this PR to see the reasoning for it.

@chrisbreiding chrisbreiding merged commit a423195 into develop Nov 22, 2023
5 checks passed
@chrisbreiding chrisbreiding deleted the crb/nohoist-devtools-protocol branch November 22, 2023 13:19
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Dec 5, 2023

Released in 13.6.1.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v13.6.1, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Dec 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants