chore: add type linting + compilation checks to packages #30776

jennifer-shehane · 2024-12-17T14:55:51Z

Additional details

We aren't running typescript linting or compilation checks in many of our packages, so this is meant to remedy some of that. There were a lot of failures.

NOTE: This does not include running tsc on the runner package since there were a lot more errors, but it is running tslint. See here: https://github.com/cypress-io/cypress/pull/30776/files#diff-3f56f0ca38e3b8dff3d2436a8f56f2cfb94120eeaa9af84182dce52d8a87d751R10

Steps to test

How has the user experience changed?

PR Tasks

Have tests been added/updated?
Has a PR for user-facing changes been opened in cypress-documentation?
Have API changes been updated in the type definitions?

cypress · 2024-12-17T15:45:01Z

cypress Run #59376

Run Properties: Passed #59376 • 1daefcfce3: Update packages/reporter/src/hooks/hook-model.ts

Project	`cypress`
Branch Review	`runner-check-ts`
Run status	`Passed #59376`
Run duration	`20m 28s`
Commit	`1daefcfce3: Update packages/reporter/src/hooks/hook-model.ts`
Committer	`Jennifer Shehane`
View all properties for this run ↗︎

Test results
Failures	`0`
Flaky	`3`
Pending	`1326`
Skipped	`0`
Passing	`29270`
View all changes introduced in this branch ↗︎

UI Coverage `46.11%`
Untested elements	`189`
Tested elements	`166`

Accessibility `92.54%`
Failed rules	`3 critical` `8 serious` `2 moderate` `2 minor`
Failed elements	`906`

… into runner-check-ts

…tion

jennifer-shehane · 2024-12-20T18:29:13Z

packages/app/src/specs/SpecsListBanners.cy.tsx

+          defaultMessages.specPage.banners.componentTesting.title = ctBannerTitle.replace('{0}', 'React')
+        }
+
+        cy.wrap(Object.entries(defaultMessages.specPage.banners[camelCase(bannerTestId)])).each((entry) => {


I added this assertion because after my updates, the rendering of the banner content was broken, but I only detected this through Percy. We should have a test that asserts on the content for the future.

cacieprins

Looks good! Most comments are about noting why the floating promises rule is disabled when it is, some potentially unnecessary async/await patterns, and unwrapping .thens in async functions. Nothing show-stopping!

cacieprins · 2025-01-02T18:20:15Z

packages/app/src/composables/useSpecFilter.ts

  })

  // initialize spec filter in store
+  // tslint:disable:no-floating-promises


Can you add a comment about why the floating promise rule is disabled?

cacieprins · 2025-01-02T18:48:05Z

packages/driver/cypress/e2e/commands/net_stubbing.cy.ts

@@ -1749,6 +1749,7 @@ describe('network stubbing', { retries: 15 }, function () {
      }).as('create')

      cy.then(() => {
+        // tslint:disable:no-floating-promises


Can the fetch() be returned? That way the cy.then will wait for the promise to be resolved ... unless that introduces flake?

cacieprins · 2025-01-02T18:49:23Z

packages/driver/cypress/e2e/cypress/proxy-logging.cy.ts

@@ -50,6 +50,7 @@ describe('Proxy Logging', () => {
      // TODO(webkit): fix+unskip for webkit release
      browser: '!webkit',
    }, (done) => {
+      // tslint:disable:no-floating-promises
      fetch('/some-url')


would cy.wrap cause issues with the test?

cacieprins · 2025-01-02T18:50:50Z

packages/driver/cypress/e2e/e2e/origin/cypress_api.cy.ts

@@ -212,6 +212,7 @@ describe('cy.origin Cypress API', { browser: '!webkit' }, () => {
      })

      cy.origin('http://www.foobar.com:3500', () => {
+        // tslint:disable:no-floating-promises


cy.then(Cypress.session.clearCurrentSessionData) maybe?

cacieprins · 2025-01-02T18:52:57Z

packages/driver/cypress/e2e/e2e/origin/patches.cy.ts

@@ -309,6 +309,7 @@ describe('src/cross-origin/patches', { browser: '!webkit', defaultCommandTimeout
              let url = new URL('/test-request', 'http://app.foobar.com:3500').toString()

              return new Promise<void>((resolve, reject) => {
+                // tslint:disable:no-floating-promises
                fetch(url, {


fetch itself returns a promise, creating a promise here isn't really necessary, just return fetch()

cacieprins · 2025-01-02T18:55:40Z

packages/driver/src/cy/commands/navigation.ts

    // only send up page loading events when we're
    // not stable!
-    stabilityChanged(Cypress, state, config, bool)
+    await stabilityChanged(Cypress, state, config, bool)


changing to async/await can add an unnecessary promise layer - will just return stabilityChanged(..etc) work?

packages/driver/src/cy/commands/origin/validator.ts

cacieprins · 2025-01-02T19:02:41Z

packages/socket/lib/cdp-browser.ts

@@ -22,14 +22,14 @@ export class CDPBrowserSocket extends Emitter implements SocketShape {

    this._namespace = namespace

-    const send = (payload: string) => {
+    const send = async (payload: string) => {


This may be a little easier to read:

const send = async (payload: string) => { const parsed = JSON.parse(payload) const [event, callbackEvent, args] = await decode(parsed) super.emit(event, ...args) await this.emit(callbackEvent) }

cacieprins · 2025-01-02T19:03:30Z

packages/socket/lib/cdp-browser.ts

@@ -65,7 +66,7 @@ export class CDPBrowserSocket extends Emitter implements SocketShape {
      this.once(uuid, callback)
    }

-    encode([event, uuid, args], this._namespace).then((encoded: any) => {
+    await encode([event, uuid, args], this._namespace).then((encoded: any) => {


same as above - since this fn is changing to async/await, it's a good time to unwrap the .then

cacieprins · 2025-01-02T19:04:24Z

packages/web-config/tsconfig.json

@@ -1,7 +1,7 @@
 {
  "compilerOptions": {
    /* Basic Options */
-    "target": "es2015",
+    "target": "es2017",


es2022 doesn't work here?

packages/driver/src/cy/commands/sessions/origins.ts

ryanthemanuel · 2025-01-03T14:34:10Z

packages/icons/scripts/build.ts

@@ -24,4 +24,5 @@ async function build () {
  await fs.remove(iconsetPath)
 }

+// tslint:disable:no-floating-promises
 build()


We could do something like this for better error handling here (and to get rid of the tslint:disable)

Suggested change

build()

build().then(() => {

console.log('icons successfully built')

}).catch((error) => {

console.log('icons failed to build', error)

})

ryanthemanuel · 2025-01-03T14:34:58Z

packages/icons/scripts/ico.ts

@@ -11,6 +11,7 @@ const files = [
  fs.readFileSync('./assets/icons/icon_256x256.png'),
 ]

+// tslint:disable:no-floating-promises


Same thing here. Could add some error handling.

packages/reporter/src/hooks/hook-model.ts

Co-authored-by: Ryan Manuel <[email protected]>

chore: add type linting + compilation checks to runner package

bd7156e

jennifer-shehane self-assigned this Dec 17, 2024

empty commit

b539409

A bunch of tslint fixes

20f5552

jennifer-shehane changed the title ~~chore: add type linting + compilation checks to runner package~~ chore: add type linting + compilation checks to packages Dec 17, 2024

jennifer-shehane added 16 commits December 17, 2024 16:26

Merge branch 'develop' into runner-check-ts

9a9a471

wow it is building

37f6367

Merge branch 'runner-check-ts' of https://github.com/cypress-io/cypress…

3f5dff5

… into runner-check-ts

Fix issue with CT not mounting correctly with comments within it

498080a

Fix net-stubbing.ct.ts failures

58d11e6

Fix tslint: disable comment

91ca342

move target into compilerOptions

a6f8043

fix tslint disable comment

685d816

update proxy-logging to undo changes

9db9e96

standardize the tslint:disable comments

dd574ba

fix comment

192b266

fix the banner content not displaying and write a test for this situa…

59493e2

…tion

fix ct reference

c2c5f71

put target to es2020

01fa9cc

actually set the property with replaced title

1251de8

Merge branch 'develop' into runner-check-ts

717fa52

jennifer-shehane commented Dec 20, 2024

View reviewed changes

jennifer-shehane requested a review from cacieprins January 2, 2025 15:46

cacieprins approved these changes Jan 2, 2025

View reviewed changes

jennifer-shehane requested a review from ryanthemanuel January 2, 2025 21:12

ryanthemanuel reviewed Jan 3, 2025

View reviewed changes

packages/driver/src/cy/commands/sessions/origins.ts Show resolved Hide resolved

ryanthemanuel reviewed Jan 3, 2025

View reviewed changes

packages/reporter/src/hooks/hook-model.ts Outdated Show resolved Hide resolved

jennifer-shehane and others added 2 commits January 3, 2025 09:38

Update packages/reporter/src/hooks/hook-model.ts

1daefcf

Co-authored-by: Ryan Manuel <[email protected]>

Merge branch 'develop' into runner-check-ts

38a9257

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: add type linting + compilation checks to packages #30776

chore: add type linting + compilation checks to packages #30776

jennifer-shehane commented Dec 17, 2024 •

edited

Loading

cypress bot commented Dec 17, 2024 •

edited

Loading

jennifer-shehane Dec 20, 2024

cacieprins left a comment •

edited

Loading

cacieprins Jan 2, 2025

cacieprins Jan 2, 2025

cacieprins Jan 2, 2025

cacieprins Jan 2, 2025

cacieprins Jan 2, 2025

cacieprins Jan 2, 2025

cacieprins Jan 2, 2025

cacieprins Jan 2, 2025

cacieprins Jan 2, 2025

ryanthemanuel Jan 3, 2025

ryanthemanuel Jan 3, 2025

-build()
+build().then(() => {
+  console.log('icons successfully built')
+}).catch((error) => {
+  console.log('icons failed to build', error)
+})

chore: add type linting + compilation checks to packages #30776

Are you sure you want to change the base?

chore: add type linting + compilation checks to packages #30776

Conversation

jennifer-shehane commented Dec 17, 2024 • edited Loading

Additional details

Steps to test

How has the user experience changed?

PR Tasks

cypress bot commented Dec 17, 2024 • edited Loading

Choose a reason for hiding this comment

cacieprins left a comment • edited Loading

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

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

jennifer-shehane commented Dec 17, 2024 •

edited

Loading

cypress bot commented Dec 17, 2024 •

edited

Loading

cacieprins left a comment •

edited

Loading