Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(store): debounce selectSignal by default #2190

Merged
merged 6 commits into from
Jul 16, 2024

Conversation

arturovt
Copy link
Member

@arturovt arturovt commented Jul 11, 2024

In this commit, we make selectSignal debouncable by default. This means that changes originating from the state stream will not immediately update the signal but will instead occur as soon as possible (in the next microtask tick). By decoupling state signal updates from synchronous changes, we adhere to constraints on updates within effects. There are situations where actions dispatched within effects are synchronous, causing the state signal to also receive synchronous updates. This scenario can lead to an error indicating that signal writes are not permitted within effects.

Resolves #2180

In this commit, we make `selectSignal` debouncable by default. This means that changes
originating from the state stream will not immediately update the signal but will instead
occur as soon as possible (in the next microtask tick). By decoupling state signal updates
from synchronous changes, we adhere to constraints on updates within effects. There are
situations where actions dispatched within effects are synchronous, causing the state signal
to also receive synchronous updates. This scenario can lead to an error indicating that signal
writes are not permitted within effects.
Copy link

nx-cloud bot commented Jul 11, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 18f8ede. 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

Sent with 💌 from NxCloud.

packages/store/src/store.ts Outdated Show resolved Hide resolved
Copy link

bundlemon bot commented Jul 11, 2024

BundleMon (Integration Projects)

Files updated (2)
Status Path Size Limits
Main bundles(Gzip)
hello-world-ng17/dist-integration/main.(hash)
.js
69.64KB (+187B +0.26%) +1%
Main bundles(Gzip)
hello-world-ng16/dist-integration/main.(hash)
.js
68.61KB (+143B +0.2%) +1%

Total files change +330B +0.23%

Final result: ✅

View report in BundleMon website ➡️


Current branch size history | Target branch size history

@arturovt arturovt force-pushed the feat/debounce-select-signal branch from 79249d4 to 929f08a Compare July 12, 2024 12:43
@arturovt arturovt force-pushed the feat/debounce-select-signal branch from 929f08a to c7977ba Compare July 12, 2024 12:44
} catch {
return toSignal(observable, { initialValue, injector: this._injector });
}
return toSignal(observable, { initialValue });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nice, so this is able to determine the correct injection context at run time🎉.

Copy link

codeclimate bot commented Jul 16, 2024

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

The test coverage on the diff in this pull request is 92.3% (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.

@arturovt arturovt merged commit 0108915 into master Jul 16, 2024
14 checks passed
@arturovt arturovt deleted the feat/debounce-select-signal branch July 16, 2024 16:12
@markwhitfeld markwhitfeld added this to the v18.1.0 milestone Jul 30, 2024
@simfyz
Copy link

simfyz commented Aug 2, 2024

@arturovt @markwhitfeld This is causing issues when using accessing signals inside dispatch. the signal value becomes null. Mostly when the default is null and values are set by an API call.

panda = select(ZooState.panda);

ngOnInit() {
    this.store.dispatch(new GetAnimals).pipe(takeUntilDestroyed(this.destroyRef)).subscribe(() => {
        const cutePanda = panda(); // value() is undefined here
    });
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐞[BUG]: Dispatching an Action within an effect causes an error
4 participants