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.

Sign up for GitHub

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: Remove pkg/driver //@ts-nocheck part 3 #19837

Merged
merged 25 commits into from
Feb 8, 2022
Merged

chore: Remove pkg/driver //@ts-nocheck part 3 #19837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
7024b42
jquery
sainthkh Jan 24, 2022
d6a566b
connectors
sainthkh Jan 24, 2022
32473f3
cookies.ts
sainthkh Jan 24, 2022
9ba036f
exec.ts
sainthkh Jan 24, 2022
782f4d7
files.ts
sainthkh Jan 24, 2022
3169800
fixtures.ts
sainthkh Jan 24, 2022
35ddaa6
location.ts
sainthkh Jan 24, 2022
0db42d1
misc.ts
sainthkh Jan 24, 2022
eaeee33
navigation.ts
sainthkh Jan 24, 2022
f05575e
check.ts
sainthkh Jan 24, 2022
d20e0ae
trigger.ts
sainthkh Jan 24, 2022
b6e2787
type.ts
sainthkh Jan 24, 2022
642956c
browser.ts
sainthkh Jan 24, 2022
42101c9
chai_jquery.ts
sainthkh Jan 24, 2022
f404f16
commands.ts
sainthkh Jan 24, 2022
89a1f91
cookies.ts
sainthkh Jan 24, 2022
330913f
error_utils.ts
sainthkh Jan 24, 2022
8e4291e
location.ts
sainthkh Jan 24, 2022
61ed8c5
fix error
sainthkh Feb 3, 2022
4b9b7aa
Add note.
sainthkh Feb 3, 2022
b47480f
feedbacks.
sainthkh Feb 3, 2022
3ff094b
Merge branch 'develop' into remove-ts-nocheck-part3
emilyrohrbough Feb 3, 2022
a7e4e96
chore(contributing): fix network_error_handling_spec for external con…
flotwig Feb 7, 2022
d9a91d3
Merge branch 'develop' into remove-ts-nocheck-part3
flotwig Feb 7, 2022
154dca5
Merge branch 'develop' into remove-ts-nocheck-part3
emilyrohrbough Feb 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions packages/driver/src/config/jquery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
// @ts-nocheck

import $ from 'jquery'
import JQuery from 'jquery'
import _ from 'lodash'

import { scrollTo } from './jquery.scrollto'
import $dom from '../dom'

// Add missing types.
interface ExtendedJQueryStatic extends JQueryStatic {
Copy link
Member

Choose a reason for hiding this comment

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

Won't this caused typescript errors for individuals using JQuery methods other than find and expr?

Copy link
Member

Choose a reason for hiding this comment

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

nvm. this isn't the types provided via the cli! sorry aboutt that

find: any
expr: JQuery.Selectors & { filters: any }
}

const $: ExtendedJQueryStatic = JQuery as any

// force jquery to have the same visible
// and hidden logic as cypress

Expand Down
11 changes: 5 additions & 6 deletions packages/driver/src/cy/commands/actions/check.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @ts-nocheck
import _ from 'lodash'
import Promise from 'bluebird'

Expand All @@ -7,7 +6,7 @@ import $utils from '../../../cypress/utils'
import $errUtils from '../../../cypress/error_utils'
import $elements from '../../../dom/elements'

const checkOrUncheck = (Cypress, cy, type, subject, values = [], userOptions = {}) => {
const checkOrUncheck = (Cypress, cy, type, subject, values: any[] = [], userOptions = {}) => {
// we're not handling conversion of values to strings
// in case we've received numbers

Expand All @@ -18,15 +17,15 @@ const checkOrUncheck = (Cypress, cy, type, subject, values = [], userOptions = {
values = []
} else {
// make sure we're an array of values
values = [].concat(values)
values = ([] as any[]).concat(values)
}

// keep an array of subjects which
// are potentially reduced down
// to new filtered subjects
const matchingElements = []
const matchingElements: HTMLElement[] = []

const options = _.defaults({}, userOptions, {
const options: Record<string, any> = _.defaults({}, userOptions, {
$el: subject,
log: true,
force: false,
Expand Down Expand Up @@ -75,7 +74,7 @@ const checkOrUncheck = (Cypress, cy, type, subject, values = [], userOptions = {
matchingElements.push(el)
}

const consoleProps = {
const consoleProps: Record<string, any> = {
'Applied To': $dom.getElements($el),
'Elements': $el.length,
}
Expand Down
4 changes: 1 addition & 3 deletions packages/driver/src/cy/commands/actions/trigger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// @ts-nocheck

import _ from 'lodash'
import Promise from 'bluebird'

Expand Down Expand Up @@ -59,7 +57,7 @@ export default (Commands, Cypress, cy, state, config) => {

({ options: userOptions, position, x, y } = $actionability.getPositionFromArguments(positionOrX, y, userOptions))

const options = _.defaults({}, userOptions, {
const options: Record<string, any> = _.defaults({}, userOptions, {
log: true,
$el: subject,
bubbles: true,
Expand Down
12 changes: 8 additions & 4 deletions packages/driver/src/cy/commands/actions/type.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @ts-nocheck
import _ from 'lodash'
import Promise from 'bluebird'

Expand All @@ -15,7 +14,11 @@ const debug = debugFn('cypress:driver:command:type')
export default function (Commands, Cypress, cy, state, config) {
const { keyboard } = cy.devices

function type (subject, chars, options = {}) {
// Note: These "change type of `any` to X" comments are written instead of changing them directly
// because Cypress extends user-given options with Cypress internal options.
// These comments will be removed after removing `// @ts-nocheck` comments in `packages/driver`.
// TODO: change the type of `any` to `Partial<Cypress.TypeOptions>`
function type (subject, chars, options: any = {}) {
Copy link
Member

Choose a reason for hiding this comment

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

What is preventing you from making this change now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cypress often extends then user-given options with some internal values like in line 24-34.

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

Lines 24 to 34 in bc87d6e

options = _.defaults({}, userOptions, {
$el: subject,
log: true,
verify: true,
force: false,
delay: config('keystrokeDelay') || $Keyboard.getConfig().keystrokeDelay,
release: true,
parseSpecialCharSequences: true,
waitForAnimations: config('waitForAnimations'),
animationDistanceThreshold: config('animationDistanceThreshold'),
})

And TypeScript treats it as a type error. That's why it's commented.

By the way, I tried to rename user-given options and internal options as opts in #6459. But it was rejected and refactored in the current style. Maybe we need to find a way to name our internal option objects to solve this problem.

Copy link
Member

Choose a reason for hiding this comment

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

Can you add a small notes summarizing what you explained here? It makes sense to me but might not be obvious to others without doing a deeper drive into the code base on the work required here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem is that this comment exists on almost every command. So it's a bit hard to find the right place.

And I'll remove all of them after removing // @ts-nocheck comments. (I believe the part 4 will be the final.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I estimated when removing TODOs will be done. And it might take 1-2 months in current speed. So, I decided to add a note above.

I guess we need this somewhere in our code.

const userOptions = options
let updateTable

Expand Down Expand Up @@ -366,7 +369,7 @@ export default function (Commands, Cypress, cy, state, config) {
// Firefox sends a click event automatically.
if (!Cypress.isBrowser('firefox')) {
const ctor = $dom.getDocumentFromElement(el).defaultView?.PointerEvent
const event = new ctor('click')
const event = new ctor!('click')

el.dispatchEvent(event)
}
Expand Down Expand Up @@ -510,7 +513,8 @@ export default function (Commands, Cypress, cy, state, config) {
})
}

function clear (subject, options = {}) {
// TODO: change the type of `any` to `Partial<ClearOptions>`
function clear (subject, options: any = {}) {
const userOptions = options

options = _.defaults({}, userOptions, {
Expand Down
18 changes: 9 additions & 9 deletions packages/driver/src/cy/commands/connectors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
// @ts-nocheck

import _ from 'lodash'
import Promise from 'bluebird'

import $dom from '../../dom'
import $utils from '../../cypress/utils'
import $errUtils from '../../cypress/error_utils'
import $errUtils, { CypressError } from '../../cypress/error_utils'

const returnFalseIfThenable = (key, ...args) => {
const returnFalseIfThenable = (key, ...args): boolean => {
if ((key === 'then') && _.isFunction(args[0]) && _.isFunction(args[1])) {
// https://github.com/cypress-io/cypress/issues/111
// if we're inside of a promise then the promise lib will naturally
Expand All @@ -22,6 +20,8 @@ const returnFalseIfThenable = (key, ...args) => {

return false
}

return true
}

const primitiveToObject = (memo) => {
Expand Down Expand Up @@ -181,7 +181,7 @@ export default function (Commands, Cypress, cy, state) {

const invokeFn = (subject, userOptionsOrStr, ...args) => {
const userOptionsPassed = _.isObject(userOptionsOrStr) && !_.isFunction(userOptionsOrStr)
let userOptions = null
let userOptions: Record<string, any> | null = null
let str = null

if (!userOptionsPassed) {
Expand Down Expand Up @@ -219,7 +219,7 @@ export default function (Commands, Cypress, cy, state) {

const message = getMessage()

let traversalErr = null
let traversalErr: CypressError | null = null

// copy userOptions because _log is added below.
const options = _.extend({}, userOptions)
Expand Down Expand Up @@ -568,7 +568,7 @@ export default function (Commands, Cypress, cy, state) {
return ret
}

return thenFn(el, userOptions, callback, state)
return thenFn(el, userOptions, callback)
}

// generate a real array since bluebird is finicky and
Expand All @@ -586,9 +586,9 @@ export default function (Commands, Cypress, cy, state) {
// cy.resolve + cy.wrap are upgraded to handle
// promises
Commands.addAll({ prevSubject: 'optional' }, {
then () {
then (subject, userOptions, fn) {
// eslint-disable-next-line prefer-rest-params
return thenFn.apply(this, arguments)
return thenFn.apply(this, [subject, userOptions, fn])
},
})

Expand Down
19 changes: 11 additions & 8 deletions packages/driver/src/cy/commands/cookies.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// @ts-nocheck

import _ from 'lodash'
import Promise from 'bluebird'

Expand Down Expand Up @@ -110,7 +108,7 @@ export default function (Commands, Cypress, cy, state, config) {
})
}

const getAndClear = (log, timeout, options = {}) => {
const getAndClear = (log?, timeout?, options = {}) => {
Copy link
Member

Choose a reason for hiding this comment

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

Why is log optional here? it's not optional for automateCookies

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it's called without them at line 164.

cypress/packages/driver/src/cy/commands/cookies.ts

Line 164 in bc87d6e

return getAndClear()

return automateCookies('get:cookies', options, log, timeout)
.then((resp) => {
// bail early if we got no cookies!
Expand Down Expand Up @@ -166,7 +164,8 @@ export default function (Commands, Cypress, cy, state, config) {
})

return Commands.addAll({
getCookie (name, options = {}) {
// TODO: change the type of `any` to `Partial<Cypress.Loggable & Cypress.Timeoutable>`
getCookie (name, options: any = {}) {
const userOptions = options

options = _.defaults({}, userOptions, {
Expand Down Expand Up @@ -212,7 +211,8 @@ export default function (Commands, Cypress, cy, state, config) {
.catch(handleBackendError('getCookie', 'reading the requested cookie from', onFail))
},

getCookies (options = {}) {
// TODO: change the type of `any` to `Partial<Cypress.Loggable & Cypress.Timeoutable>`
getCookies (options: any = {}) {
const userOptions = options

options = _.defaults({}, userOptions, {
Expand Down Expand Up @@ -250,7 +250,8 @@ export default function (Commands, Cypress, cy, state, config) {
.catch(handleBackendError('getCookies', 'reading cookies from', options._log))
},

setCookie (name, value, options = {}) {
// TODO: change the type of `any` to `Partial<Cypress.SetCookieOptions>`
setCookie (name, value, options: any = {}) {
const userOptions = options

options = _.defaults({}, userOptions, {
Expand Down Expand Up @@ -331,7 +332,8 @@ export default function (Commands, Cypress, cy, state, config) {
}).catch(handleBackendError('setCookie', 'setting the requested cookie in', onFail))
},

clearCookie (name, options = {}) {
// TODO: change the type of `any` to `Partial<Cypress.Loggable & Cypress.Timeoutable>`
clearCookie (name, options: any = {}) {
const userOptions = options

options = _.defaults({}, userOptions, {
Expand Down Expand Up @@ -380,7 +382,8 @@ export default function (Commands, Cypress, cy, state, config) {
.catch(handleBackendError('clearCookie', 'clearing the requested cookie in', onFail))
},

clearCookies (options = {}) {
// TODO: change the type of `any` to `Partial<Cypress.Loggable & Cypress.Timeoutable>`
clearCookies (options: any = {}) {
const userOptions = options

options = _.defaults({}, userOptions, {
Expand Down
5 changes: 2 additions & 3 deletions packages/driver/src/cy/commands/exec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
// @ts-nocheck

import _ from 'lodash'
import Promise from 'bluebird'

import $errUtils from '../../cypress/error_utils'

export default (Commands, Cypress, cy) => {
Commands.addAll({
exec (cmd, options = {}) {
// TODO: change the type of `any` to `Partical<Cypress.ExecOptions>`
exec (cmd, options: any = {}) {
const userOptions = options

options = _.defaults({}, userOptions, {
Expand Down
7 changes: 4 additions & 3 deletions packages/driver/src/cy/commands/files.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
// @ts-nocheck
import _ from 'lodash'
import { basename } from 'path'

import $errUtils from '../../cypress/error_utils'

export default (Commands, Cypress, cy, state) => {
Commands.addAll({
readFile (file, encoding, options = {}) {
// TODO: change the type of `any` to `Partial<Cypress.Loggable & Cypress.Timeoutable>`
readFile (file, encoding, options: any = {}) {
let userOptions = options

if (_.isObject(encoding)) {
Expand Down Expand Up @@ -109,7 +109,8 @@ export default (Commands, Cypress, cy, state) => {
return verifyAssertions()
},

writeFile (fileName, contents, encoding, options = {}) {
// TODO: change the type of `any` to `Partial<Cypress.WriteFileOPtions & Cypress.Timeoutable>`
writeFile (fileName, contents, encoding, options: any = {}) {
let userOptions = options

if (_.isObject(encoding)) {
Expand Down
4 changes: 1 addition & 3 deletions packages/driver/src/cy/commands/fixtures.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// @ts-nocheck

import _ from 'lodash'
import Promise from 'bluebird'
import { basename } from 'path'
Expand Down Expand Up @@ -44,7 +42,7 @@ export default (Commands, Cypress, cy, state, config) => {
return Promise.resolve(clone(resp))
}

let options = {}
let options: Record<string, any> = {}

if (_.isObject(args[0])) {
options = args[0]
Expand Down
8 changes: 4 additions & 4 deletions packages/driver/src/cy/commands/location.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// @ts-nocheck

import _ from 'lodash'
import Promise from 'bluebird'

Expand All @@ -8,7 +6,8 @@ const { throwErrByPath } = $errUtils

export default (Commands, Cypress, cy) => {
Commands.addAll({
url (options = {}) {
// TODO: change the type of `any` to `Partial<Cypress.UrlOptions>`
url (options: any = {}) {
const userOptions = options

options = _.defaults({}, userOptions, { log: true })
Expand Down Expand Up @@ -39,7 +38,8 @@ export default (Commands, Cypress, cy) => {
return resolveHref()
},

hash (options = {}) {
// TODO: change the type of `any` to `Partial<Cypress.Loggable & Cypress.Timeoutable>`
hash (options: any = {}) {
const userOptions = options

options = _.defaults({}, userOptions, { log: true })
Expand Down
4 changes: 2 additions & 2 deletions packages/driver/src/cy/commands/misc.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @ts-nocheck
import _ from 'lodash'
import Promise from 'bluebird'

Expand Down Expand Up @@ -50,7 +49,8 @@ export default (Commands, Cypress, cy, state) => {
return null
},

wrap (arg, options = {}) {
// TODO: change the type of `any` to `Partial<Cypress.Loggable & Cypress.Timeoutable>`
wrap (arg, options: any = {}) {
const userOptions = options

options = _.defaults({}, userOptions, {
Expand Down
Loading