ngxs / store

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

refactor(store): add action registry #2224

Closed arturovt closed 1 month ago

arturovt commented 1 month ago

This adds a new internal construct, called the ActionRegistry. The ActionRegistry facilitates the registration, and recall, of all handlers associated with an action type. The default handler registered for each @Action within a state is now responsible for handling the return value of the user's handler within the state and the cancellation logic for the handler.

nx-cloud[bot] commented 1 month ago

☁️ Nx Cloud Report

CI is running/has finished running commands for commit ddbed5600f4e8746aafdef919f1ebe379d2dbc8d. 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/QIhdo6XRlz?utm_source=pull-request&utm_medium=comment) - [`nx run-many --target=test --all --configuration=ci --maxWorkers=4`](https://cloud.nx.app/runs/NXjSNjeQbX?utm_source=pull-request&utm_medium=comment) - [`nx lint-types store`](https://cloud.nx.app/runs/7c2griHmSw?utm_source=pull-request&utm_medium=comment) - [`nx run-many --target=build --all`](https://cloud.nx.app/runs/Z1FyF0c2vJ?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@2224.tgz ```

@ngxs/form-plugin

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

@ngxs/hmr-plugin

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

@ngxs/router-plugin

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

@ngxs/storage-plugin

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

@ngxs/store

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

@ngxs/websocket-plugin

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

commit: ddbed56

bundlemon[bot] commented 1 month ago

BundleMon

Files updated (1) Status | Path | Size | Limits :------------: | ------------ | :------------: | :------------: :white_check_mark: | fesm2022/ngxs-store.mjs
| 100.32KB (+152B +0.15%) | 101KB / +0.5%
Unchanged files (5) Status | Path | Size | Limits :------------: | ------------ | :------------: | :------------: :white_check_mark: | fesm2022/ngxs-store-internals.mjs
| 11.36KB | 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 +152B +0.12%

Groups updated (2) Status | Path | Size | Limits :------------: | ------------ | :------------: | :------------: :white_check_mark: | @ngxs/store(esm2022)[gzip]
./esm2022/**/*.mjs
| 222.11KB (+1.27KB +0.58%) | +1% :white_check_mark: | @ngxs/store(fesm2022)[gzip]
./fesm2022/*.mjs
| 30.63KB (-87B -0.28%) | +1%

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 (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.09KB | +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.76KB | +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-ng18/dist-integration/browser/mai
n-(hash).js
| 71.81KB (+64B +0.09%) | +1% :white_check_mark: | Main bundles(Gzip)
hello-world-ng16/dist-integration/main.(hash)
.js
| 67.74KB (+47B +0.07%) | +1% :white_check_mark: | Main bundles(Gzip)
hello-world-ng17/dist-integration/main.(hash)
.js
| 68.71KB (+47B +0.07%) | +1%

Total files change +158B +0.07%

Final result: :white_check_mark:

View report in BundleMon website ➡️


Current branch size history | Target branch size history

codeclimate[bot] commented 1 month ago

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

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

This pull request will bring the total coverage in the repository to 95.3% (-0.1% change).

View more on Code Climate.

hakimio commented 1 month ago

@arturovt Sorry to bother you, just wanted to ask for a small favor. @angular-ru/ngxs library relies heavily on ngxs/store internals. After your recent ngxs refactoring, I have updated @angular-ru/ngxs to be compatible with ngxs v18.1.3 (https://github.com/Angular-RU/angular-ru-sdk/pull/1527) but since I'm not too familiar with ngxs inner workings, I imagine the code might be suboptimal. When you have some time, could you take a look at the PR and specifically at the @DataAction decorator (docs) and let me know if the code could be improved?

arturovt commented 1 month ago

I would first remove NgxsDataInjector.ngZone?.run(...) because it’s not required for NGXS. It doesn’t care whether dispatched events come from the Angular zone or not, as the state updates are decoupled from the dispatcher, and the new state always occurs within the Angular zone.

Since states are decorated with @StateRepository, that decorator could return a new constructor function, which is always called within the Angular injection context. You can capture the injector through this[SomeInternalInjectorSymbol] = inject(Injector). The descriptor.value function now has access to this[SomeInternalInjectorSymbol] and can retrieve anything from the "app" injector as well as from the globally configured static injector. However, static properties wouldn't work properly for server-side rendered apps. If a new app is rendered for the first HTTP request and then a second request comes in, the newly bootstrapped app would overwrite static properties.

hakimio commented 1 month ago

Thanks for the suggestions.

You can capture the injector through this[SomeInternalInjectorSymbol] = inject(Injector).

Basically this type of injector usage instead of global static injector could fix the mentioned SSR issue?

What do you think about the operation caching used in the @DataAction decorator? Is it necessary and can it be simplified?

Is it ok to use hydrateActionMetasMap() in the decorator or could there be some better/simpler way to register the handlers?

arturovt commented 4 weeks ago

I think it might be better to find a way to register actions without relying on internal APIs, which are likely to change in the future. It seems that the DataAction decorator updates the metadata only when the method is called for the first time, but it should behave like the Action decorator and update the metadata immediately. This would eliminate the need to call hydrateActionMetasMap.

hakimio commented 4 weeks ago

@arturovt thank you for your insights. The @Action decorator is definitely a good example of how @DataAction could be implemented.