-
Notifications
You must be signed in to change notification settings - Fork 403
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
fix(store): use computed
in selectSignal
#2201
Conversation
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 0a67567. 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 targetsSent with 💌 from NxCloud. |
BundleMon (Integration Projects)Files updated (2)
Total files change -2.13KB -1.54% Final result: ✅ View report in BundleMon website ➡️ |
f752bf2
to
4b54fda
Compare
Note: it is necessary to revert these changes and cease experimenting with decoupling signal changes. Previously, an issue was reported where calling `dispatch` from within an effect caused an error indicating that signal writes are not allowed. We attempted to resolve this by decoupling state signal changes through debouncing them, ensuring that consumers always received signal updates asynchronously. However, we should no longer pursue this approach because it fixes initial issues while introducing new ones. It is the responsibility of developers to ensure a valid reactive context when updating signals. The `dispatch` function can be wrapped in `untracked`, as it is common practice to disallow updates to ANY signal within effects, not just NGXS state. Debouncing signals disrupts zoneless server-side rendering because Angular is unaware of tasks scheduled by the `debounceTime` operator. Consequently, content is serialized before the `debounceTime` timer fires internally, which updates the signal and allows the view to reflect the updated value.
4b54fda
to
7922a1b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there may be a better way to resolve this.
Let's pair on this to come to a good solution.
PS. Why were the tests for packages/store/tests/select-signal-on-provider-levels.spec.ts
removed?
I'm wondering if by reverting to this first solution, updates to the state from an effect will not propagate to the components when doing zoneless. I need to check if the use of PS. I have an idea that may work for all cases. I'll investigate tomorrow morning and then we can discuss. |
Code Climate has analyzed commit 0a67567 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 95.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. |
Note: it is necessary to revert these changes and cease experimenting
with decoupling signal changes.
Previously, an issue was reported where calling
dispatch
from within aneffect caused an error indicating that signal writes are not allowed. We attempted
to resolve this by decoupling state signal changes through debouncing them, ensuring
that consumers always received signal updates asynchronously. However, we should no
longer pursue this approach because it fixes initial issues while introducing new ones.
It is the responsibility of developers to ensure a valid reactive context when updating
signals. The
dispatch
function can be wrapped inuntracked
, as it is common practiceto disallow updates to ANY signal within effects, not just NGXS state.
Debouncing signals disrupts zoneless server-side rendering because Angular is unaware
of tasks scheduled by the
debounceTime
operator. Consequently, content is serializedbefore the
debounceTime
timer fires internally, which updates the signal and allowsthe view to reflect the updated value.