address comments from code review

cypress-io · Jun 24, 2024 · 001813e · 001813e
1 parent 4b1e648
commit 001813e
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 11 deletions.
diff --git a/npm/angular-signals/src/mount.ts b/npm/angular-signals/src/mount.ts
@@ -31,7 +31,6 @@ import type { Subscription } from 'rxjs'
  * Additional module configurations needed while mounting the component, like
  * providers, declarations, imports and even component @Inputs()
  *
- *
  * @interface MountConfig
  * @see https://angular.io/api/core/testing/TestModuleMetadata
  */
@@ -267,7 +266,8 @@ function setupFixture<T> (
 }
 
 // Best known way to currently detect whether or not a function is a signal is if the signal symbol exists.
-// From there, we can take our best guess based on what exists on the object itself
+// From there, we can take our best guess based on what exists on the object itself.
+// @see https://github.com/cypress-io/cypress/issues/29731.
 function isSignal (prop: any): boolean {
   try {
     const symbol = Object.getOwnPropertySymbols(prop).find((symbol) => symbol.toString() === 'Symbol(SIGNAL)')
@@ -280,17 +280,20 @@ function isSignal (prop: any): boolean {
   }
 }
 
-// currently not a great way to detect if a function is an InputSignal. When we discover a better way to detect a signal we should incorporate it here.
+// currently not a great way to detect if a function is an InputSignal.
+// @see https://github.com/cypress-io/cypress/issues/29731.
 function isInputSignal (prop: any): boolean {
   return isSignal(prop) && typeof prop === 'function' && prop['name'] === 'inputValueFn'
 }
 
-// currently not a great way to detect if a function is a Model Signal. Need to improve this.
+// currently not a great way to detect if a function is a Model Signal.
+// @see https://github.com/cypress-io/cypress/issues/29731.
 function isModelSignal (prop: any): boolean {
   return isSignal(prop) && isWritableSignal(prop) && typeof prop.subscribe === 'function'
 }
 
-// currently not a great way to detect if a function is a Writable Signal. Need to improve this.
+// currently not a great way to detect if a function is a Writable Signal.
+// @see https://github.com/cypress-io/cypress/issues/29731.
 function isWritableSignal (prop: any): boolean {
   return isSignal(prop) && typeof prop === 'function' && typeof prop.set === 'function'
 }

diff --git a/npm/angular/src/mount.ts b/npm/angular/src/mount.ts
@@ -29,7 +29,6 @@ import {
  * Additional module configurations needed while mounting the component, like
  * providers, declarations, imports and even component @Inputs()
  *
- *
  * @interface MountConfig
  * @see https://angular.io/api/core/testing/TestModuleMetadata
  */

diff --git a/system-tests/projects/angular-signals/.gitignore b/system-tests/projects/angular-signals/.gitignore
@@ -1 +1 @@
-.angular
+.angular
diff --git a/system-tests/projects/angular-signals/cypress/support/component-index.html b/system-tests/projects/angular-signals/cypress/support/component-index.html
@@ -9,4 +9,4 @@
   <body>
     <div data-cy-root></div>
   </body>
-</html>
+</html>
diff --git a/system-tests/projects/angular-signals/src/signals-required/signals-required.component.cy.ts b/system-tests/projects/angular-signals/src/signals-required/signals-required.component.cy.ts
@@ -25,9 +25,10 @@ it('can handle input signal as primitive value (title prop)', () => {
   cy.get('[data-cy="signals-required-component-title-display"]').should('contain.text', 'Signals Component as Primitive')
 })
 
-// FIXME: we currently need to allow this as there isn't a great way to set input signals in a component due to how input signals are instantiated
-// This also should lead to a better testing experience where the input signal can actually be asserted, though not possible within the constaints
-// of the regular angular framework
+// FIXME: we currently need to allow this as there isn't a great way to set input signals in a component due to how input signals are instantiated.
+// This also should lead to a better testing experience where the input signal can actually be asserted, though not possible within the constraints
+// of the regular angular framework.
+// @see https://github.com/cypress-io/cypress/issues/29732.
 it('also allows writable signal as input signal', () => {
   cy.mount(SignalsRequiredComponent, {
     componentProperties: {