chore: reverting #22742 (#23047)

* Revert "chore: Refactor cy.state('subject') and `cy.then()` (#22742)"

This reverts commit 0ed8dd5.

* Run more tests

circle.yml

@@ -27,7 +27,7 @@ mainBuildFilters: &mainBuildFilters
     branches:
       only:
         - develop
-        - tbiethman/electron-19
+        - revert-22742
 
 # usually we don't build Mac app - it takes a long time
 # but sometimes we want to really confirm we are doing the right thing
@@ -36,7 +36,7 @@ macWorkflowFilters: &darwin-workflow-filters
   when:
     or:
     - equal: [ develop, << pipeline.git.branch >> ]
-    - equal: [ 'tbiethman/electron-19', << pipeline.git.branch >> ]
+    - equal: [ 'revert-22742', << pipeline.git.branch >> ]
     - matches:
           pattern: "-release$"
           value: << pipeline.git.branch >>
@@ -45,7 +45,7 @@ linuxArm64WorkflowFilters: &linux-arm64-workflow-filters
   when:
     or:
     - equal: [ develop, << pipeline.git.branch >> ]
-    - equal: [ 'tbiethman/electron-19', << pipeline.git.branch >> ]
+    - equal: [ 'revert-22742', << pipeline.git.branch >> ]
     - matches:
           pattern: "-release$"
           value: << pipeline.git.branch >>
@@ -129,7 +129,7 @@ commands:
       - run:
           name: Check current branch to persist artifacts
           command: |
-            if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "tbiethman/electron-19" ]]; then
+            if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "revert-22742" ]]; then
               echo "Not uploading artifacts or posting install comment for this branch."
               circleci-agent step halt
             fi
@@ -883,7 +883,7 @@ commands:
               fi
 
               curl -L https://raw.githubusercontent.com/cypress-io/cypress/$branch/scripts/ensure-node.sh --output ci-ensure-node.sh
-            else 
+            else
               # if no .node-version file exists, we no-op the node script and use the global yarn
               echo '' > ci-ensure-node.sh
             fi

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

@@ -806,7 +806,7 @@ describe('src/cy/commands/assertions', () => {
       })
 
       cy.get('body').then(() => {
-        expect(cy.currentSubject()).to.match('body')
+        expect(cy.state('subject')).to.match('body')
       })
     })
 

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

@@ -58,8 +58,7 @@ describe('src/cy/commands/commands', () => {
       cy
       .get('input:first')
       .parent()
-      .command('login', 'brian@foo.com')
-      .then(($input) => {
+      .command('login', 'brian@foo.com').then(($input) => {
         expect($input.get(0)).to.eq(input.get(0))
       })
     })

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

@@ -158,16 +158,15 @@ describe('src/cy/commands/connectors', () => {
         })
       })
 
-      it('should pass the eventually resolved thenable value downstream', () => {
+      it('should pass the eventual resolved thenable value downstream', () => {
         cy
         .wrap({ foo: 'bar' })
         .then((obj) => {
           return cy
           .wait(10)
           .then(() => {
             return obj.foo
-          })
-          .then((value) => {
+          }).then((value) => {
             expect(value).to.eq('bar')
 
             return value
@@ -310,7 +309,7 @@ describe('src/cy/commands/connectors', () => {
             return $div
           })
           .then(function () {
-            expect(cy.currentSubject()).not.to.be.instanceof(this.remoteWindow.$)
+            expect(cy.state('subject')).not.to.be.instanceof(this.remoteWindow.$)
           })
         })
       })
@@ -424,6 +423,7 @@ describe('src/cy/commands/connectors', () => {
 
           cy.get('div:first').invoke('parent').then(function ($parent) {
             expect($parent).to.be.instanceof(this.remoteWindow.$)
+            expect(cy.state('subject')).to.match(parent)
           })
         })
       })

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

@@ -133,16 +133,6 @@ describe('driver/src/cypress/cy', () => {
         expect(userInvocationStack).to.include('.cy.js')
       })
     })
-
-    it('supports cy.state(\'subject\') for backwards compatability', () => {
-      cy.stub(Cypress.utils, 'warning')
-      const a = {}
-
-      cy.wrap(a).then(() => {
-        expect(cy.state('subject')).to.equal(a)
-        expect(Cypress.utils.warning).to.be.calledWith('`cy.state(\'subject\')` has been deprecated and will be removed in a future release. Consider migrating to `cy.currentSubject()` instead.')
-      })
-    })
   })
 
   context('custom commands', () => {

packages/driver/src/cross-origin/origin_fn.ts

@@ -105,7 +105,7 @@ export const handleOriginFn = (Cypress: Cypress.Cypress, cy: $Cy) => {
       queueFinished = true
       setRunnableStateToPassed()
       Cypress.specBridgeCommunicator.toPrimary('queue:finished', {
-        subject: cy.currentSubject(),
+        subject: cy.state('subject'),
       }, {
         syncGlobals: true,
       })

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

@@ -100,9 +100,47 @@ export default function (Commands, Cypress, cy, state) {
 
     cy.once('command:enqueued', enqueuedCommand)
 
+    // this code helps juggle subjects forward
+    // the same way that promises work
+    const current = state('current')
+    const next = current.get('next')
+
+    // TODO: this code may no longer be necessary
+    // if the next command is chained to us then when it eventually
+    // runs we need to reset the subject to be the return value of the
+    // previous command so the subject is continuously juggled forward
+    if (next && next.get('chainerId') === current.get('chainerId')) {
+      const checkSubject = (newSubject, args) => {
+        if (state('current') !== next) {
+          return
+        }
+
+        // get whatever the previous commands return
+        // value is. this likely does not match the 'var current'
+        // command in the case of nested cy commands
+        const s = next.get('prev').get('subject')
+
+        // find the new subject and splice it out
+        // with our existing subject
+        const index = _.indexOf(args, newSubject)
+
+        if (index > -1) {
+          args.splice(index, 1, s)
+        }
+
+        return cy.removeListener('next:subject:prepared', checkSubject)
+      }
+
+      cy.on('next:subject:prepared', checkSubject)
+    }
+
     const getRet = () => {
       let ret = fn.apply(ctx, args)
 
+      if (cy.isCy(ret)) {
+        ret = undefined
+      }
+
       if (ret && invokedCyCommand && !ret.then) {
         $errUtils.throwErrByPath('then.callback_mixes_sync_and_async', {
           onFail: options._log,
@@ -123,11 +161,6 @@ export default function (Commands, Cypress, cy, state) {
         return subject
       }
 
-      // If the user callback returned a non-null value, we break cypress' subject chaining
-      // logic, so that we can use this subject as-is rather than the subject generated by
-      // any chainers inside the callback (if any exist).
-      cy.breakSubjectLinksToCurrentChainer()
-
       return ret
     }).catch(Promise.TimeoutError, () => {
       return $errUtils.throwErrByPath('invoke_its.timed_out', {
@@ -465,16 +498,54 @@ export default function (Commands, Cypress, cy, state) {
         $errUtils.throwErrByPath('each.invalid_argument')
       }
 
-      if (subject?.length === undefined) {
+      const nonArray = () => {
         return $errUtils.throwErrByPath('each.non_array', {
           args: { subject: $utils.stringify(subject) },
         })
       }
 
+      try {
+        if (!('length' in subject)) {
+          nonArray()
+        }
+      } catch (e) {
+        nonArray()
+      }
+
       if (subject.length === 0) {
         return subject
       }
 
+      // if we have a next command then we need to
+      // slice in this existing subject as its subject
+      // due to the way we queue promises
+      const next = state('current').get('next')
+
+      if (next) {
+        const checkSubject = (newSubject, args, firstCall) => {
+          if (state('current') !== next) {
+            return
+          }
+
+          // https://github.com/cypress-io/cypress/issues/4921
+          // When dual commands like contains() is used as the firstCall (cy.contains() style),
+          // we should not prepend subject.
+          if (!firstCall) {
+            // find the new subject and splice it out
+            // with our existing subject
+            const index = _.indexOf(args, newSubject)
+
+            if (index > -1) {
+              args.splice(index, 1, subject)
+            }
+          }
+
+          return cy.removeListener('next:subject:prepared', checkSubject)
+        }
+
+        cy.on('next:subject:prepared', checkSubject)
+      }
+
       let endEarly = false
 
       const yieldItem = (el, index) => {
@@ -502,16 +573,11 @@ export default function (Commands, Cypress, cy, state) {
 
       // generate a real array since bluebird is finicky and
       // doesnt want an 'array-like' structure like jquery instances
+      // need to take into account regular arrays here by first checking
+      // if its an array instance
       return Promise
       .each(_.toArray(subject), yieldItem)
-      .then(() => {
-        // cy.each does *not* want to use any subjects that the user's callback generated - therefore we break
-        // cypress' subject chaining logic, which by default would override this with any subjects generated by
-        // the callback function.
-        cy.breakSubjectLinksToCurrentChainer()
-
-        return subject
-      })
+      .return(subject)
     },
   })
 

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

@@ -32,7 +32,7 @@ export default (Commands, Cypress, cy, state) => {
         const restoreCmdIndex = state('index') + 1
 
         cy.queue.insert(restoreCmdIndex, $Command.create({
-          args: [cy.currentSubject()],
+          args: [state('subject')],
           name: 'log-restore',
           fn: (subject) => subject,
         }))

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

@@ -35,9 +35,7 @@ export default (Commands, Cypress, cy, state) => {
 
     fn.call(cy.state('ctx'), subject)
 
-    const cleanup = () => {
-      cy.removeListener('command:start', setWithinSubject)
-    }
+    const cleanup = () => cy.removeListener('command:start', setWithinSubject)
 
     // we need a mechanism to know when we should remove
     // our withinSubject so we dont accidentally keep it
@@ -78,10 +76,6 @@ export default (Commands, Cypress, cy, state) => {
       })
     }
 
-    // TODO: Rework cy.within to use chainer-based subject chaining, rather than its custom withinSubject state.
-    // For now, we leave this logic in place and just ensure that the new rules don't interfere with it.
-    cy.breakSubjectLinksToCurrentChainer()
-
     return subject
   }
 

packages/driver/src/cy/logGroup.ts

@@ -27,7 +27,6 @@ export default (Cypress, userOptions: Cypress.LogGroup.Config, fn: Cypress.LogGr
   const endLogGroupCmd = $Command.create({
     name: 'end-logGroup',
     injected: true,
-    args: [],
   })
 
   const forwardYieldedSubject = () => {

packages/driver/src/cypress.ts

@@ -218,24 +218,6 @@ class $Cypress {
     _.extend(this, browserInfo(config))
 
     this.state = $SetterGetter.create({}) as unknown as StateFunc
-
-    /*
-     * As part of the Detached DOM effort, we're changing the way subjects are determined in Cypress.
-     * While we usually consider cy.state() to be internal, in the case of cy.state('subject'),
-     * cypress-testing-library, one of our most popular plugins, relies on it.
-     * https://github.com/testing-library/cypress-testing-library/blob/1af9f2f28b2ca62936da8a8acca81fc87e2192f7/src/utils.js#L9
-     *
-     * Therefore, we've added this shim to continue to support them. The library is actively maintained, so this
-     * shouldn't need to stick around too long (written 07/22).
-     */
-    Object.defineProperty(this.state(), 'subject', {
-      get: () => {
-        $errUtils.warnByPath('subject.state_subject_deprecated')
-
-        return cy.currentSubject()
-      },
-    })
-
     this.originalConfig = _.cloneDeep(config)
     this.config = $SetterGetter.create(config, (config) => {
       if (this.isCrossOriginSpecBridge ? !window.__cySkipValidateConfig : !window.top!.__cySkipValidateConfig) {
@@ -658,6 +640,9 @@ class $Cypress {
       case 'cy:url:changed':
         return this.emit('url:changed', args[0])
 
+      case 'cy:next:subject:prepared':
+        return this.emit('next:subject:prepared', ...args)
+
       case 'cy:collect:run:state':
         return this.emitThen('collect:run:state')
 

packages/driver/src/cypress/chainer.ts

@@ -4,13 +4,22 @@ import $stackUtils from './stack_utils'
 export class $Chainer {
   specWindow: Window
   chainerId: string
+  firstCall: boolean
 
   constructor (specWindow) {
     this.specWindow = specWindow
     // 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
   }
 
   static remove (key) {