- Notifications
You must be signed in to change notification settings - Fork1.1k
fix(flake.nix): synchronize playwright version in nix and package.json#16715
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
fix(flake.nix): synchronize playwright version in nix and package.json#16715
Uh oh!
There was an error while loading.Please reload this page.
Conversation
ThomasK33 commentedFeb 26, 2025
This stack of pull requests is managed byGraphite. Learn more aboutstacking. |
810cd08 to2da521bCompare
johnstcn left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Downgrading the version of playwright insite/package.json means we're missing the fixes for four regressions fixed in v1.47.1 and v1.47.2:
- https://github.com/microsoft/playwright/releases/tag/v1.47.1
- https://github.com/microsoft/playwright/releases/tag/v1.47.2
Is there any other alternative to this approach?
ThomasK33 commentedFeb 26, 2025
Are we affected by those regressions? The alternative is to upgrade |
johnstcn commentedFeb 26, 2025
mafredri commentedFeb 26, 2025
I simply updated to the latest one at the time, so no deeper intention. In general I'd prefer upgrade over downgrade, but no idea what changes have gone in. Regressions from going .2 to .0 don't seem relevant IMO but don't know if we use |
aslilac commentedFeb 26, 2025
I don't know much about nix, but I really don't like the idea of not being able to use newer versions of a package from an entirely different package manager just because nix says so |
ThomasK33 commentedFeb 27, 2025
This PR focuses not on locking down the Playwright version but ensuring that the browsers required by a specific Playwright version are already pre-installed in the image. If the browsers are not pre-installed, Playwright attempts to download them from the internet, which can negatively impact startup times, bandwidth usage, and reproducibility. I am OK with upgrading to a newer Playwright version, but the constraint of pre-installing the browsers into the image is firm. |
johnstcn commentedFeb 27, 2025
So this constraint should only affect users of the Nix image? IMO it's a reasonable constraint. |
ThomasK33 commentedFeb 27, 2025
Yes, it causes the Nix image build to exit early if the Nix Playwright version does not match the one specified in package.json, as it cannot guarantee that Interestingly, I couldn't find us pre-caching or downloading the browsers in the standard dogfood image, which leads me to conclude that we are re-downloading Google Chrome and friends each time a workspace starts? |
2da521b to860863bCompareaslilac commentedFeb 27, 2025
does it only affect nix users tho? it places a constraint on what version of playwright we can place in package.json, which affects everyone
not on workspace start for everyone, you either have to remember to download them before running the e2e tests or put the download command in your ~/personalize script |
ThomasK33 commentedFeb 27, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Maybe I misunderstand our startup script, but we explicitly call coder/dogfood/contents/main.tf Line 354 incccdf1e
|
aslilac commentedFeb 27, 2025
oh nevermind! I didn't know we added that |
Change-Id: I8cc78e842f7d0b1d2a90a4517a186a03636c5559Signed-off-by: Thomas Kosiewski <tk@coder.com>
860863b toab42451CompareThomasK33 commentedFeb 28, 2025
Yeah, I just double-checked, and it indeed reinstalls system dependencies and Chromium at each workspace start due to that startup script. I've refactored this behavior by removing the This also means we're pinning the Playwright version in the Dockerfile, as we cannot access the site's package.json at build time. |
9ded2cc intomainUh oh!
There was an error while loading.Please reload this page.

Uh oh!
There was an error while loading.Please reload this page.
Ensure that the version of Playwright installed with the Nix flake is equal to the one specified in
site/package.json.-- This assertion ensures thatpnpm playwright:installwill not attempt to download newer browser versions not present in the Nix image, fixing the startup script and reducing the startup time, aspnpm playwright:installwill not download or install anything.We also pre-install the required Playwright web browsers in the dogfood Dockerfile. This change prevents us from redownloading system dependencies and Google Chrome each time a workspace starts.
Change-Id: I8cc78e842f7d0b1d2a90a4517a186a03636c5559
Signed-off-by: Thomas Kosiewskitk@coder.com