chore(deps): Updating electron to v18 + node v16.13.2 #21418

tbiethman · 2022-05-10T06:59:29Z

User facing changelog

Upgraded electron from 15.5.1 to 18.0.4.
Upgraded bundled Node.js version from 16.5.0 to 16.13.2.
Upgraded bundled Chromium version from 94.0.4606.81 to 100.0.4896.75.

Additional details

The Upgrading Electron guide was followed to produce this PR.

I included inline comments where additional clarification was needed. A few changes of note:

Issues were found with some of the origin related tests in newer versions of Firefox. An issue was logged, and the affected tests have been tweaked to pass in the meantime without being outright skipped.
The workaround added with fix: Improve Big Sur browser detection performance #17807 in regards to browser detection performance has been removed, as the Electron issue causing the slowdown has been corrected.

How has the user experience changed?

There shouldn't be any functional difference in the user experience beyond any changes arising from the bundled Chromium update. If other differences are seen, please raise them.

PR Tasks

Have tests been added/updated?
Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
[n/a] Has a PR for user-facing changes been opened in cypress-documentation?
[n/a] Have API changes been updated in the type definitions?
[n/a] Have new configuration options been added to the cypress.schema.json?

cypress-bot · 2022-05-10T06:59:32Z

Thanks for taking the time to open a PR!

Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

tbiethman · 2022-05-10T07:02:30Z

circle.yml

@@ -1247,7 +1257,7 @@ jobs:

  runner-integration-tests-firefox:
    <<: *defaults
-    resource_class: medium
+    resource_class: medium+


The resources were bumped for the Firefox tests due to increased memory consumption. #21319 was logged to investigate further.

tbiethman · 2022-05-10T07:04:16Z

circle.yml

@@ -896,7 +904,9 @@ commands:
            yarn binary-build --platform $PLATFORM --version $(node ./scripts/get-next-version.js)
      - run:
          name: Zip the binary
-          command: yarn binary-zip --platform $PLATFORM
+          command: |
+            apt-get update && apt-get install -y zip


zip was removed from our base image generator in a recent slimming down effort. We can add it back/generate a new image if there's a concern with pulling it in here.

tbiethman · 2022-05-10T07:13:32Z

packages/driver/cypress/integration/commands/actions/trigger_spec.js

        cy.spy(cy, 'ensureElementIsNotAnimating')

-        cy.get('button:first').trigger('tap', { animationDistanceThreshold: 1000 }).then(() => {
+        cy.get('button:first').trigger('tap', { animationDistanceThreshold: 1000 }).then(($btn) => {
+          const { fromElWindow } = Cypress.dom.getElementCoordinatesByPosition($btn)


These tests related to animationDistanceThreshold across trigger/type command tests were failing in a similar way in Firefox.

Firefox reports positions/scroll offsets with something like 13 digits of precision. We do a lot rounding in our coordinate logic that could be impacted by this. These tests also seem to be influenced by how the AUT is being scaled; I can get them to pass/fail in develop by running the tests with the AUT scaled at different sizes.

I updated these tests to grab the reference element window coordinates after the element has been scrolled into position, which makes us less susceptible to variances within Firefox's reported scroll position. This allows them to pass consistently without need to add 1px buffers to our assertions.

I wonder if we can use a similar technique to remove this 1px hack for Firefox in 10.0-release? https://github.com/cypress-io/cypress/blob/10.0-release/packages/driver/cypress/e2e/commands/actions/type.cy.js#L360-L377

tbiethman · 2022-05-10T07:16:30Z

packages/driver/cypress/integration/e2e/origin/commands/navigation.spec.ts

      cy.reload()
-      cy.get(':checkbox[name="colors"][value="blue"]').should('not.be.checked')
+
+      cy.window().should('not.have.prop', 'cy_testCustomValue', true)


Firefox now caches input values between reloads, causing this test to fail. Updated to use a window prop that we can be certain will be reset between reloads.

tbiethman · 2022-05-10T07:20:02Z

packages/web-config/tsconfig.json

@@ -56,7 +56,8 @@
    "allowSyntheticDefaultImports": true,
    "esModuleInterop": true,
    "noErrorTruncation": true,
-    "experimentalDecorators": true
+    "experimentalDecorators": true,
+    "skipLibCheck": true


web-config was the only package that didn't have skipLibCheck enabled. I attempted to update our node types to v16 but it seems to cause more problems than it fixed; #21329 was logged to dedicate more time this effort.

web-config is used in several packages btw. What motivated this change?

Bumping to electron 18 caused yarn type-check to fail for this package due to conflicts in some deep transient dependencies. I assume this was due to electron's bumping to @types/node@16: https://github.com/cypress-io/cypress/pull/21418/files#diff-51e4f558fae534656963876761c95b83b6ef5da5103c4adef6768219ed76c2deR8720-R8723. Looks like we might have been inadvertently(?) resolving against v15 before?

My type-config-fu is admittedly weak. It seems like we can resolve the failures if I add an explicit @types/node@14 dependency to this package. Would that be preferred (in the short term, I know going to 16 across the board would be ideal)? My understanding was that this flag forced a check on types we are actually referencing rather than on all dependencies.

Hmm, this seems fine after reading the config doc: https://www.typescriptlang.org/tsconfig#skipLibCheck I don't think you need to change it. The doc says we can use Yarn resolutions maybe to pin it to 14 everywhere, but again, I don't think it's really an issue.

lmiller1990 · 2022-05-10T13:00:35Z

I wonder if this will solve #15932 🤔

cypress · 2022-05-10T13:59:24Z

Test summary

4434 • 0 • 54 • 0 • 1

Run details


Project	cypress
Status	Passed
Commit	`9a2326a`
Started	May 12, 2022 3:15 PM
Ended	May 12, 2022 3:27 PM
Duration	11:58 💡
OS	Linux Debian - 10.11
Browser	Firefox 98

View run in Cypress Dashboard ➡️

Flakiness

	cypress/integration/commands/actions/click_spec.js		1
1	... > scroll-behavior > can scroll to and click elements in html with scroll-behavior: smooth

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

tbiethman · 2022-05-10T15:08:11Z

@lmiller1990 I was still able to reproduce #15932 😞 :

tbiethman · 2022-05-10T17:01:13Z

packages/driver/cypress/integration/e2e/origin/origin.spec.ts

          done()
        })

        const variable = () => {}

-        cy.origin('http://idp.com:3500', { args: variable }, (variable) => {
+        cy.origin('http://foobar.com:3500', { args: variable }, (variable) => {


This spec would hang across all browsers when visiting idp.com; switching to foobar.com like all other specs in this file caused the test to pass (well, fail in the way we expect it too). This is odd, but not sure if it's indicative of a larger problem or the same problem recorded in #21300.

tbiethman · 2022-05-10T18:36:46Z

Note: we're currently pinned to electron 18.0.4 because 18.1.0 introduced a breaking change that has since been fixed (within the last day): electron/electron#34150

We should be able to consume the 18.2.2 release when it goes out.

AkechiShiro · 2022-05-10T19:56:23Z

Any news when this dependency bump will happen, I'm just asking for some security related matters (the old dependencies have a few minor issues, nothing really critical)

Maybe soon, in a few weeks or it is still being at an early work of being adopted in the main branch ? Needs a few months of work still ?

tbiethman · 2022-05-10T21:12:58Z

@AkechiShiro I'd anticipate these dependency bumps to be included in our next 9.x release in two weeks.

lmiller1990

Great to see us finally updating electron. I left one comment about a potential update we can make in 10.0-release, that will be landing soon.

lmiller1990 · 2022-05-10T23:21:00Z

packages/driver/cypress/integration/commands/actions/trigger_spec.js

        cy.spy(cy, 'ensureElementIsNotAnimating')

-        cy.get('button:first').trigger('tap', { animationDistanceThreshold: 1000 }).then(() => {
+        cy.get('button:first').trigger('tap', { animationDistanceThreshold: 1000 }).then(($btn) => {
+          const { fromElWindow } = Cypress.dom.getElementCoordinatesByPosition($btn)


I wonder if we can use a similar technique to remove this 1px hack for Firefox in 10.0-release? https://github.com/cypress-io/cypress/blob/10.0-release/packages/driver/cypress/e2e/commands/actions/type.cy.js#L360-L377

packages/driver/cypress/integration/e2e/origin/config_env.spec.ts

…essary.

flotwig

Really nice work @tbiethman, I know this was a bear of an upgrade.

Will this be a separate PR to close #21236?

packages/driver/src/cy/commands/actions/type.ts

flotwig · 2022-05-11T17:33:51Z

packages/launcher/lib/darwin/util.ts

 import * as path from 'path'
 import * as plist from 'plist'
-import * as semver from 'semver'


You can remove this from the package.json now I think.

packages/launcher/lib/darwin/util.ts

flotwig · 2022-05-11T17:35:27Z

packages/launcher/lib/detect.ts

  // we can detect same browser under different aliases
  // tell them apart by the name and the version property
  if (!goalBrowsers) {
    goalBrowsers = browsers
  }

-  // BigSur (darwin 20.x) and Electron 12+ cause huge performance issues when


🎉 🎉 🎉 🎉 🎉 🎉

flotwig · 2022-05-11T17:36:43Z

packages/web-config/tsconfig.json

@@ -56,7 +56,8 @@
    "allowSyntheticDefaultImports": true,
    "esModuleInterop": true,
    "noErrorTruncation": true,
-    "experimentalDecorators": true
+    "experimentalDecorators": true,
+    "skipLibCheck": true


web-config is used in several packages btw. What motivated this change?

tbiethman · 2022-05-11T18:48:28Z

@flotwig Regarding #21236, I was planning on following up with a quick PR after merging develop into 10.0-release. I've confirmed that Electron 18 does not show the same issues that 15.5.1 does with the increased verification time.

lmiller1990 · 2022-05-12T01:17:19Z

Recording breaking four ✅ - let's ship it 😎 , unless some action around #21418 (comment) is needed?

tbiethman · 2022-05-12T17:04:55Z

@lmiller1990 That comment has been addressed, seems like it's just not showing the full convo in Zach's review summary. We're good to go.

cypress-bot · 2022-05-23T19:41:45Z

Released in 9.7.0.

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

chore: Updating electron to v18 + node v16.13.2

736ce36

tbiethman requested review from a team as code owners May 10, 2022 06:59

tbiethman requested review from jennifer-shehane and removed request for a team May 10, 2022 06:59

tbiethman removed request for a team and jennifer-shehane May 10, 2022 07:00

tbiethman self-assigned this May 10, 2022

tbiethman commented May 10, 2022

View reviewed changes

tbiethman mentioned this pull request May 10, 2022

chore(deps): update dependency electron to v18 🌟 #19090

Closed

1 task

Updating origin test, it needs to be fixed a different way

f5f14ed

tbiethman mentioned this pull request May 10, 2022

Add support for node 16.10+ #21425

Closed

tbiethman commented May 10, 2022

View reviewed changes

lmiller1990 self-requested a review May 10, 2022 23:16

lmiller1990 approved these changes May 10, 2022

View reviewed changes

mjhenkes reviewed May 11, 2022

View reviewed changes

packages/driver/cypress/integration/e2e/origin/config_env.spec.ts Outdated Show resolved Hide resolved

tbiethman mentioned this pull request May 11, 2022

feat: distinguish app vs launchpad utm_source when using utm params #21424

Merged

1 task

Removing origin workaround for config_env specs, it should not be nec…

eaf48f2

…essary.

mjhenkes approved these changes May 11, 2022

View reviewed changes

tbiethman requested a review from flotwig May 11, 2022 15:27

Merge branch 'develop' into tbiethman/chore/electron-18-update

294e968

flotwig suggested changes May 11, 2022

View reviewed changes

Removing now-unused semver dependency

e4d0e0d

tbiethman requested a review from flotwig May 11, 2022 21:39

flotwig approved these changes May 11, 2022

View reviewed changes

AtofStryker approved these changes May 11, 2022

View reviewed changes

Merge branch 'develop' into tbiethman/chore/electron-18-update

9a2326a

tbiethman merged commit 8311cf1 into develop May 12, 2022

tbiethman deleted the tbiethman/chore/electron-18-update branch May 12, 2022 17:06

cypress-bot bot locked as resolved and limited conversation to collaborators May 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(deps): Updating electron to v18 + node v16.13.2 #21418

chore(deps): Updating electron to v18 + node v16.13.2 #21418

tbiethman commented May 10, 2022 •

edited

cypress-bot bot commented May 10, 2022

tbiethman May 10, 2022

tbiethman May 10, 2022

tbiethman May 10, 2022

lmiller1990 May 10, 2022

tbiethman May 10, 2022

tbiethman May 10, 2022

flotwig May 11, 2022

tbiethman May 11, 2022 •

edited

flotwig May 11, 2022

lmiller1990 commented May 10, 2022

cypress bot commented May 10, 2022 •

edited

tbiethman commented May 10, 2022 •

edited

tbiethman May 10, 2022

tbiethman commented May 10, 2022

AkechiShiro commented May 10, 2022 •

edited

tbiethman commented May 10, 2022

lmiller1990 left a comment

lmiller1990 May 10, 2022

flotwig left a comment

flotwig May 11, 2022

flotwig May 11, 2022

flotwig May 11, 2022

tbiethman commented May 11, 2022

lmiller1990 commented May 12, 2022 •

edited

tbiethman commented May 12, 2022

cypress-bot bot commented May 23, 2022

chore(deps): Updating electron to v18 + node v16.13.2 #21418

chore(deps): Updating electron to v18 + node v16.13.2 #21418

Conversation

tbiethman commented May 10, 2022 • edited

User facing changelog

Additional details

How has the user experience changed?

PR Tasks

cypress-bot bot commented May 10, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tbiethman May 11, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmiller1990 commented May 10, 2022

cypress bot commented May 10, 2022 • edited

Test summary

Run details

Flakiness

tbiethman commented May 10, 2022 • edited

Choose a reason for hiding this comment

tbiethman commented May 10, 2022

AkechiShiro commented May 10, 2022 • edited

tbiethman commented May 10, 2022

lmiller1990 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flotwig left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tbiethman commented May 11, 2022

lmiller1990 commented May 12, 2022 • edited

tbiethman commented May 12, 2022

cypress-bot bot commented May 23, 2022

tbiethman commented May 10, 2022 •

edited

tbiethman May 11, 2022 •

edited

cypress bot commented May 10, 2022 •

edited

tbiethman commented May 10, 2022 •

edited

AkechiShiro commented May 10, 2022 •

edited

lmiller1990 commented May 12, 2022 •

edited