ngxs / store

🚀 NGXS - State Management for Angular
http://ngxs.io
MIT License
3.54k stars 405 forks source link

fix(store): prevent writing to state once action handler is unsubscribed #2231

Closed arturovt closed 1 month ago

arturovt commented 1 month ago

In this commit, we update the implementation of action invocation. The key addition is that we now prevent writing to the state whenever an action handler is unsubscribed (completed or cancelled). Since we have a "unique" state context object for each action being invoked, we can set its setState and patchState functions to no-ops, essentially making them do nothing when invoked.

nx-cloud[bot] commented 1 month ago

☁️ Nx Cloud Report

CI is running/has finished running commands for commit b2b3a9720c072416360edc153aa414e1a808c92c. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 4 targets - [`nx run-many --target=lint --all --exclude=create-app`](https://cloud.nx.app/runs/jKRX2jUp2j?utm_source=pull-request&utm_medium=comment) - [`nx run-many --target=test --all --configuration=ci --maxWorkers=4`](https://cloud.nx.app/runs/GKlgGOI6Ra?utm_source=pull-request&utm_medium=comment) - [`nx lint-types store`](https://cloud.nx.app/runs/ldGMk3uvKQ?utm_source=pull-request&utm_medium=comment) - [`nx run-many --target=build --all`](https://cloud.nx.app/runs/WDse5esTNY?utm_source=pull-request&utm_medium=comment)

Sent with 💌 from NxCloud.

pkg-pr-new[bot] commented 1 month ago

Open in Stackblitz

@ngxs/devtools-plugin

``` yarn add https://pkg.pr.new/@ngxs/devtools-plugin@2231.tgz ```

@ngxs/hmr-plugin

``` yarn add https://pkg.pr.new/@ngxs/hmr-plugin@2231.tgz ```

@ngxs/form-plugin

``` yarn add https://pkg.pr.new/@ngxs/form-plugin@2231.tgz ```

@ngxs/router-plugin

``` yarn add https://pkg.pr.new/@ngxs/router-plugin@2231.tgz ```

@ngxs/storage-plugin

``` yarn add https://pkg.pr.new/@ngxs/storage-plugin@2231.tgz ```

@ngxs/store

``` yarn add https://pkg.pr.new/@ngxs/store@2231.tgz ```

@ngxs/websocket-plugin

``` yarn add https://pkg.pr.new/@ngxs/websocket-plugin@2231.tgz ```

commit: b2b3a97

bundlemon[bot] commented 1 month ago

BundleMon

Files updated (1) Status | Path | Size | Limits :------------: | ------------ | :------------: | :------------: :x: | fesm2022/ngxs-store.mjs
| 102.45KB (+1.56KB **+1.55%**) | 103KB / +0.5%
Unchanged files (5) Status | Path | Size | Limits :------------: | ------------ | :------------: | :------------: :white_check_mark: | fesm2022/ngxs-store-internals.mjs
| 11.64KB | 13KB / +0.5% :white_check_mark: | fesm2022/ngxs-store-internals-testing.mjs
| 6.83KB | 7KB / +0.5% :white_check_mark: | fesm2022/ngxs-store-operators.mjs
| 6.22KB | 7KB / +0.5% :white_check_mark: | fesm2022/ngxs-store-plugins.mjs
| 2.04KB | 3KB / +0.5% :white_check_mark: | fesm2022/ngxs-store-experimental.mjs
| 1.4KB | 2KB / +0.5%

Total files change +1.56KB +1.21%

Groups updated (2) Status | Path | Size | Limits :------------: | ------------ | :------------: | :------------: :white_check_mark: | @ngxs/store(esm2022)[gzip]
./esm2022/**/*.mjs
| 224.5KB (+1.08KB +0.48%) | +1% :x: | @ngxs/store(fesm2022)[gzip]
./fesm2022/*.mjs
| 31.24KB (+402B **+1.27%**) | +1%

Final result: :x:

View report in BundleMon website ➡️


Current branch size history | Target branch size history

bundlemon[bot] commented 1 month ago

BundleMon (NGXS Plugins)

Unchanged files (9) Status | Path | Size | Limits :------------: | ------------ | :------------: | :------------: :white_check_mark: | Plugins(fesm2022)[gzip]
storage-plugin/fesm2022/ngxs-storage-plugin.m
js
| 4.15KB | +0.5% :white_check_mark: | Plugins(fesm2022)[gzip]
router-plugin/fesm2022/ngxs-router-plugin.mjs
| 3.2KB | +0.5% :white_check_mark: | Plugins(fesm2022)[gzip]
websocket-plugin/fesm2022/ngxs-websocket-plug
in.mjs
| 2.64KB | +0.5% :white_check_mark: | Plugins(fesm2022)[gzip]
hmr-plugin/fesm2022/ngxs-hmr-plugin.mjs
| 2.61KB | +0.5% :white_check_mark: | Plugins(fesm2022)[gzip]
form-plugin/fesm2022/ngxs-form-plugin.mjs
| 2.59KB | +0.5% :white_check_mark: | Plugins(fesm2022)[gzip]
devtools-plugin/fesm2022/ngxs-devtools-plugin
.mjs
| 2.23KB | +0.5% :white_check_mark: | Plugins(fesm2022)[gzip]
logger-plugin/fesm2022/ngxs-logger-plugin.mjs
| 2.03KB | +0.5% :white_check_mark: | Plugins(fesm2022)[gzip]
storage-plugin/fesm2022/ngxs-storage-plugin-i
nternals.mjs
| 875B | +0.5% :white_check_mark: | Plugins(fesm2022)[gzip]
router-plugin/fesm2022/ngxs-router-plugin-int
ernals.mjs
| 411B | +0.5%

No change in files bundle size

Unchanged groups (1) Status | Path | Size | Limits :------------: | ------------ | :------------: | :------------: :white_check_mark: | All Plugins(fesm2022)[gzip]
./*-plugin/fesm2022/*.mjs
| 20.71KB | +0.5%

Final result: :white_check_mark:

View report in BundleMon website ➡️


Current branch size history | Target branch size history

bundlemon[bot] commented 1 month ago

BundleMon (Integration Projects)

Files updated (3) Status | Path | Size | Limits :------------: | ------------ | :------------: | :------------: :white_check_mark: | Main bundles(Gzip)
hello-world-ng17/dist-integration/main.(hash)
.js
| 68.69KB (+49B +0.07%) | +1% :white_check_mark: | Main bundles(Gzip)
hello-world-ng16/dist-integration/main.(hash)
.js
| 67.8KB (+46B +0.07%) | +1% :white_check_mark: | Main bundles(Gzip)
hello-world-ng18/dist-integration/browser/mai
n-(hash).js
| 71.87KB (+40B +0.05%) | +1%

Total files change +135B +0.06%

Final result: :white_check_mark:

View report in BundleMon website ➡️


Current branch size history | Target branch size history

markwhitfeld commented 1 month ago

@arturovt I wasn't meaning to remove that test file, but rather to add tests that showcase the cancellation behavior by writing to the state. Potentially the action could write the log entries to the state as well as the recorder. Then the normal test will show that they are the same, but then when you dispatch an action that results in cancellation, then you will see that the state doesn't receive the writes. You might want to add an id to the action so that the triggering action is apparent in the logs.

PS. I view the "don't write to state after unsubscribe" as a different set of tests to the cancellation tests (even though they use the same mechanism).

codeclimate[bot] commented 1 month ago

Code Climate has analyzed commit b2b3a972 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 91.6% (50% is the threshold).

This pull request will bring the total coverage in the repository to 95.4% (0.0% change).

View more on Code Climate.