ngxs / store

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

feat(store): add `dispatch` utility #2143

Closed arturovt closed 3 months ago

nx-cloud[bot] commented 4 months ago

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 1c7a57efcce6783b8ecfe0c3751b6932fc8b2294. 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=test --all --configuration=ci --maxWorkers=4`](https://cloud.nx.app/runs/OHHeZSAjMt?utm_source=pull-request&utm_medium=comment) - [`nx run-many --target=lint --all`](https://cloud.nx.app/runs/9rByDPS53L?utm_source=pull-request&utm_medium=comment) - [`nx lint-types store`](https://cloud.nx.app/runs/IG6vH9FiQe?utm_source=pull-request&utm_medium=comment) - [`nx run-many --target=build --all`](https://cloud.nx.app/runs/Ql7NSWz5Af?utm_source=pull-request&utm_medium=comment)

Sent with 💌 from NxCloud.

bundlemon[bot] commented 4 months ago

BundleMon (Integration Projects)

Unchanged files (2) Status | Path | Size | Limits :------------: | ------------ | :------------: | :------------: :white_check_mark: | Main bundles(Gzip)
hello-world-ng17/dist-integration/main.(hash)
.js
| 68.48KB | +1% :white_check_mark: | Main bundles(Gzip)
hello-world-ng16/dist-integration/main.(hash)
.js
| 67.03KB | +1%

Total files change -9B -0.01%

Final result: :white_check_mark:

View report in BundleMon website ➡️


Current branch size history | Target branch size history

markwhitfeld commented 3 months ago

I think we should include the dispatch utility in the @ngxs/signals package so that it is grouped together with the other utilities that follow the "signals style". It is also easier to move something into the main package at a later stage, than it out of it. What do you think?

arturovt commented 3 months ago

I thought similarly, believing it should be a part of the signals package, mainly because those utilities are quite similar.

profanis commented 3 months ago

I think we should include the dispatch utility in the @ngxs/signals package so that it is grouped together with the other utilities that follow the "signals style". It is also easier to move something into the main package at a later stage, than it out of it. What do you think?

Although it makes sense to have select and dispatch under the same package, I feel that the dispatch should be part of the core. I am thinking of a case where a consumer is not yet about to move to signals but would love to use the dispatch utility function. Having an import like this import { dispatch } from @ngxs/signals could confuse.

markwhitfeld commented 3 months ago

I think we should include the dispatch utility in the @ngxs/signals package so that it is grouped together with the other utilities that follow the "signals style". It is also easier to move something into the main package at a later stage, than it out of it. What do you think?

Although it makes sense to have select and dispatch under the same package, I feel that the dispatch should be part of the core. I am thinking of a case where a consumer is not yet about to move to signals but would love to use the dispatch utility function. Having an import like this import { dispatch } from @ngxs/signals could confuse.

Good point @profanis. What would you propose as the ideal split of APIs to packages here?

rhutchison commented 3 months ago

you could move select to @ngxs/store and keep the @ngxs/signals library as the interop for @ngrx/signals.

select and dispatch can be used without needing @ngxs/signals. You only really need @ngxs/signals if you want to use signalStore.

profanis commented 3 months ago

you could move select to @ngxs/store and keep the @ngxs/signals library as the interop for @ngrx/signals.

select and dispatch can be used without needing @ngxs/signals. You only really need @ngxs/signals if you want to use signalStore.

I like that!

markwhitfeld commented 3 months ago

I see these utilities as generally useful, and not specific to ngrx signal store though. Keeping them all together in the signals package advocates for a specific style of declaration, to which they are all consistent.

rhutchison commented 3 months ago

I see these utilities as generally useful, and not specific to ngrx signal store though. Keeping them all together in the signals package advocates for a specific style of declaration, to which they are all consistent.

What is your recommendation?

arturovt commented 3 months ago

Merged. Can revert if we agree to do this.