chore: Refactor chainer / Commands.add for readability (#22571)

* Refactor chainer / Commands.add for readability

* Fix invoking wrong function, add comment

Author: Blue F

packages/driver/cypress/e2e/commands/commands.cy.js

@@ -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()

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

@@ -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({

packages/driver/src/cypress/chainer.ts

@@ -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) {
@@ -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
-  }
 }

packages/driver/src/cypress/commands.ts

@@ -15,7 +15,6 @@ const builtInCommands = [
 
 const reservedCommandNames = {
   addAlias: true,
-  addChainer: true,
   addCommand: true,
   addCommandSync: true,
   aliasNotFoundFor: true,
@@ -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)
       },
 
       overwrite (name, fn) {

packages/driver/src/cypress/cy.ts

@@ -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()
@@ -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)
@@ -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 }) {
@@ -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
@@ -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) {