ngxs / store

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

fix(store): update `ActionDef` new to return `any` #2149

Closed arturovt closed 3 months ago

nx-cloud[bot] commented 3 months ago

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 73df8f6b31c0992904ba168c035422ac224ed522. 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/QP7FznKsxR?utm_source=pull-request&utm_medium=comment) - [`nx run-many --target=lint --all`](https://cloud.nx.app/runs/9Hcl5L4bln?utm_source=pull-request&utm_medium=comment) - [`nx lint-types store`](https://cloud.nx.app/runs/PJCpBbBWmf?utm_source=pull-request&utm_medium=comment) - [`nx run-many --target=build --all`](https://cloud.nx.app/runs/CyK6HSBc1K?utm_source=pull-request&utm_medium=comment)

Sent with 💌 from NxCloud.

bundlemon[bot] commented 3 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%

No change in files bundle size

Final result: :white_check_mark:

View report in BundleMon website ➡️


Current branch size history | Target branch size history

arturovt commented 3 months ago

I removed TReturn from ActionDef, where it was new (...args: TArgs): TReturn, since it was redundant. By mistake, I changed it to ThisType, thinking it would infer the instance type. I now believe I just need to use any as the return type for new.

markwhitfeld commented 3 months ago

What is the problem that you are looking to solve by removing the TReturn? Or was is the motivation for removing it?

arturovt commented 3 months ago

We have code where ofActionDispatched accepts a function parameter declared as shown in the test. Attempting to access result.errorType throws an error. Previously, result was inferred as any in this specific case. The use of TReturn wasn't resolving any issues, so I intend for new() to always return any.

markwhitfeld commented 3 months ago

@arturovt this has regressed from previous type matching capabilities on the action type. I will add a commit to fix this and demonstrate the capabilities I am referring to.

markwhitfeld commented 3 months ago

@arturovt I have readded the correct typing and added tests that demonstrate the usages. Hopefully this compiles, I did this in the github editor so I didn't have the build tooling.