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: Refactor chainer / Commands.add for readability #22571

Merged
merged 2 commits into from
Jun 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/driver/cypress/e2e/commands/commands.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,13 @@ describe('src/cy/commands/commands', () => {

it('throws when attempting to add a command with the same name as an internal function', (done) => {
cy.on('fail', (err) => {
expect(err.message).to.eq('`Cypress.Commands.add()` cannot create a new command named `addChainer` because that name is reserved internally by Cypress.')
expect(err.message).to.eq('`Cypress.Commands.add()` cannot create a new command named `addCommand` because that name is reserved internally by Cypress.')
expect(err.docsUrl).to.eq('https://on.cypress.io/custom-commands')

done()
})

Cypress.Commands.add('addChainer', () => {
Cypress.Commands.add('addCommand', () => {
cy
.get('[contenteditable]')
.first()
Expand Down
13 changes: 5 additions & 8 deletions packages/driver/src/cy/commands/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,11 @@ const command = function (ctx, name, ...args) {
}

export default function (Commands, Cypress, cy) {
Commands.addChainer({
// userInvocationStack has to be passed in here, but can be ignored
command (chainer, userInvocationStack, args) {
// `...args` below is the shorthand of `args[0], ...args.slice(1)`
// TypeScript doesn't allow this.
// @ts-ignore
return command(chainer, ...args)
},
$Chainer.add('command', function (chainer, userInvocationStack, args) {
// `...args` below is the shorthand of `args[0], ...args.slice(1)`
// TypeScript doesn't allow this.
// @ts-ignore
return command(chainer, ...args)
})

Commands.addAllSync({
Expand Down
46 changes: 13 additions & 33 deletions packages/driver/src/cypress/chainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,24 @@ import _ from 'lodash'
import $stackUtils from './stack_utils'

export class $Chainer {
userInvocationStack: any
specWindow: Window
chainerId: string
firstCall: boolean
useInitialStack: boolean | null

constructor (userInvocationStack, specWindow) {
this.userInvocationStack = userInvocationStack
constructor (specWindow) {
this.specWindow = specWindow
// the id prefix needs to be unique per origin, so there are not
// The id prefix needs to be unique per origin, so there are not
// collisions when chainers created in a secondary origin are passed
// to the primary origin for the command log, etc.
this.chainerId = _.uniqueId(`ch-${window.location.origin}-`)

// firstCall is used to throw a useful error if the user leads off with a
// parent command.

// TODO: Refactor firstCall out of the chainer and into the command function,
// since cy.ts already has all the necessary information to throw this error
// without an instance variable, in one localized place in the code.
this.firstCall = true
this.useInitialStack = null
}

static remove (key) {
Expand All @@ -25,40 +28,17 @@ export class $Chainer {

static add (key, fn) {
$Chainer.prototype[key] = function (...args) {
const userInvocationStack = this.useInitialStack
? this.userInvocationStack
: $stackUtils.normalizedUserInvocationStack(
(new this.specWindow.Error('command invocation stack')).stack,
)
const userInvocationStack = $stackUtils.normalizedUserInvocationStack(
(new this.specWindow.Error('command invocation stack')).stack,
)

// call back the original function with our new args
// pass args an as array and not a destructured invocation
if (fn(this, userInvocationStack, args)) {
// no longer the first call
this.firstCall = false
}
fn(this, userInvocationStack, args)

// return the chainer so additional calls
// are slurped up by the chainer instead of cy
return this
}
}

// creates a new chainer instance
static create (key, userInvocationStack, specWindow, args) {
const chainer = new $Chainer(userInvocationStack, specWindow)

// this is the first command chained off of cy, so we use
// the stack passed in from that call instead of the stack
// from this invocation
chainer.useInitialStack = true

// since this is the first function invocation
// we need to pass through onto our instance methods
const chain = chainer[key].apply(chainer, args)

chain.useInitialStack = false

return chain
}
}
14 changes: 3 additions & 11 deletions packages/driver/src/cypress/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ const builtInCommands = [

const reservedCommandNames = {
addAlias: true,
addChainer: true,
addCommand: true,
addCommandSync: true,
aliasNotFoundFor: true,
Expand Down Expand Up @@ -255,16 +254,9 @@ export default {
})
},

addChainer (obj) {
// perp loop
for (let name in obj) {
const fn = obj[name]

cy.addChainer(name, fn)
}

// prevent loop comprehension
return null
addSelector (name, fn) {
// TODO: Add overriding stuff.
return cy.addSelector(name, fn)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, whoops, slight code-splatter here, had meant to remove this before committing. I'll clean it up in an upcoming refactor. Not used yet, shouldn't be in yet. Same with selectorFns below.

},

overwrite (name, fn) {
Expand Down
102 changes: 51 additions & 51 deletions packages/driver/src/cypress/cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ export class $Cy extends EventEmitter2 implements ITimeouts, IStability, IAssert

private testConfigOverride: TestConfigOverride
private commandFns: Record<string, Function> = {}
private selectorFns: Record<string, Function> = {}

constructor (specWindow: SpecWindow, Cypress: ICypress, Cookies: ICookies, state: StateFunc, config: ICypress['config']) {
super()
Expand All @@ -244,7 +245,6 @@ export class $Cy extends EventEmitter2 implements ITimeouts, IStability, IAssert
this.stop = this.stop.bind(this)
this.reset = this.reset.bind(this)
this.addCommandSync = this.addCommandSync.bind(this)
this.addChainer = this.addChainer.bind(this)
this.addCommand = this.addCommand.bind(this)
this.now = this.now.bind(this)
this.replayCommandsFrom = this.replayCommandsFrom.bind(this)
Expand Down Expand Up @@ -675,9 +675,21 @@ export class $Cy extends EventEmitter2 implements ITimeouts, IStability, IAssert
}
}

addChainer (name, fn) {
// add this function to our chainer class
return $Chainer.add(name, fn)
runQueue () {
cy.queue.run()
.then(() => {
const onQueueEnd = cy.state('onQueueEnd')

if (onQueueEnd) {
onQueueEnd()
}
})
.catch(() => {
// errors from the queue are propagated to cy.fail by the queue itself
// and can be safely ignored here. omitting this catch causes
// unhandled rejections to be logged because Bluebird sees a promise
// chain with no catch handler
})
}

addCommand ({ name, fn, type, prevSubject }) {
Expand Down Expand Up @@ -711,17 +723,45 @@ export class $Cy extends EventEmitter2 implements ITimeouts, IStability, IAssert
}
}

cy[name] = function (...args) {
const userInvocationStack = $stackUtils.captureUserInvocationStack(cy.specWindow.Error)
const callback = (chainer, userInvocationStack, args) => {
const { firstCall, chainerId } = chainer

// dont enqueue / inject any new commands if
// onInjectCommand returns false
const onInjectCommand = cy.state('onInjectCommand')
const injected = _.isFunction(onInjectCommand)

if (injected) {
if (onInjectCommand.call(cy, name, ...args) === false) {
return
}
}

cy.enqueue({
name,
args,
type,
chainerId,
userInvocationStack,
injected,
fn: wrap(firstCall),
})

chainer.firstCall = false
}

$Chainer.add(name, callback)

cy[name] = function (...args) {
cy.ensureRunnable(name)

// this is the first call on cypress
// so create a new chainer instance
const chain = $Chainer.create(name, userInvocationStack, cy.specWindow, args)
const chainer = new $Chainer(cy.specWindow)

// store the chain so we can access it later
cy.state('chain', chain)
const userInvocationStack = $stackUtils.captureUserInvocationStack(cy.specWindow.Error)

callback(chainer, userInvocationStack, args)

// if we are in the middle of a command
// and its return value is a promise
Expand Down Expand Up @@ -753,51 +793,11 @@ export class $Cy extends EventEmitter2 implements ITimeouts, IStability, IAssert
cy.warnMixingPromisesAndCommands()
}

cy.queue.run()
.then(() => {
const onQueueEnd = cy.state('onQueueEnd')

if (onQueueEnd) {
onQueueEnd()
}
})
.catch(() => {
// errors from the queue are propagated to cy.fail by the queue itself
// and can be safely ignored here. omitting this catch causes
// unhandled rejections to be logged because Bluebird sees a promise
// chain with no catch handler
})
cy.runQueue()
}

return chain
return chainer
}

return this.addChainer(name, (chainer, userInvocationStack, args) => {
const { firstCall, chainerId } = chainer

// dont enqueue / inject any new commands if
// onInjectCommand returns false
const onInjectCommand = cy.state('onInjectCommand')
const injected = _.isFunction(onInjectCommand)

if (injected) {
if (onInjectCommand.call(cy, name, ...args) === false) {
return
}
}

cy.enqueue({
name,
args,
type,
chainerId,
userInvocationStack,
injected,
fn: wrap(firstCall),
})

return true
})
}

now (name, ...args) {
Expand Down