Skip to content

Commit

Permalink
make sure to clean up rxjs observables in order to prevent a memory leak
Browse files Browse the repository at this point in the history
  • Loading branch information
AtofStryker committed Jun 20, 2024
1 parent 3f93d59 commit 12c7e36
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 3 deletions.
1 change: 1 addition & 0 deletions npm/angular-signals/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
"@angular/common": ">=17.2",
"@angular/core": ">=17.2",
"@angular/platform-browser-dynamic": ">=17.2",
"rxjs": ">=7.5.0",
"zone.js": ">=0.13.0"
},
"files": [
Expand Down
21 changes: 18 additions & 3 deletions npm/angular-signals/src/mount.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
setupHooks,
getContainerEl,
} from '@cypress/mount-utils'
import type { Subscription } from 'rxjs'

/**
* Additional module configurations needed while mounting the component, like
Expand Down Expand Up @@ -77,6 +78,7 @@ export interface MountConfig<T> extends TestModuleMetadata {
}

let activeFixture: ComponentFixture<any> | null = null
let activeInternalSubscriptions: Subscription[] = []

function cleanup () {
// Not public, we need to call this to remove the last component from the DOM
Expand All @@ -89,8 +91,15 @@ function cleanup () {
throw notSupportedError
}

// clean up internal subscriptions if any exist. We use this for two-way data binding for
// signal() models
activeInternalSubscriptions.forEach((subscription) => {
subscription.unsubscribe()
})

getTestBed().resetTestingModule()
activeFixture = null
activeInternalSubscriptions = []
}

/**
Expand Down Expand Up @@ -323,11 +332,17 @@ function convertPropertyToSignalIfApplicable (propValue: any, componentValue: an
})

// update the model signal with the properties updates
toObservable(propValue, {
const convertedToObservable = toObservable(propValue, {
injector,
}).subscribe((value) => {
componentValue.set(value)
})

// push the subscription into an array to be cleaned up at the end of the test
// to prevent a memory leak
activeInternalSubscriptions.push(
convertedToObservable.subscribe((value) => {
componentValue.set(value)
}),
)
} else {
// it's a non signal type, set it as we only need to handle updating the model signal and emit changes on this through the output spy.
componentValue.set(propValue)
Expand Down

0 comments on commit 12c7e36

Please sign in to comment.