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

RxState.property$ should emit stable reference #6462

Closed
wants to merge 1 commit into from

Conversation

kilbot
Copy link
Contributor

@kilbot kilbot commented Oct 5, 2024

This PR shows a failing test for RxState.property$ not being strictly equal for initial and subsequent subscriptions. I believe RxDocument gets around this issue by using an internal property cache.

This PR contains:

IMPROVED TESTS

Describe the problem you have without this PR

#6459

Todos

  • Tests
  • Documentation
  • Typings
  • Changelog

@pubkey
Copy link
Owner

pubkey commented Oct 6, 2024

Ah, I understand the problem now with your rerenders. Working on a fix..

@kilbot
Copy link
Contributor Author

kilbot commented Oct 6, 2024

Thank you! There's no rush for me, the previous version works perfectly fine.

If it's helpful, I could create a version of the react example which uses observable-hooks for Suspense and reactive components.

@kilbot kilbot changed the title RxState.property$ should be stable reference RxState.property$ should emit stable reference Oct 6, 2024
@pubkey
Copy link
Owner

pubkey commented Oct 6, 2024

If it's helpful, I could create a version of the react example which uses observable-hooks for Suspense and reactive components.

No you do not have to. The unit test is enough and your example would anyway not be in the CI test suite.

pubkey added a commit that referenced this pull request Oct 6, 2024
* FIX RxState.property$ should emit stable reference #6462

* ADD changelog
@pubkey
Copy link
Owner

pubkey commented Oct 6, 2024

Fixed here #6463

@pubkey pubkey closed this Oct 6, 2024
pubkey added a commit that referenced this pull request Oct 6, 2024
* FIX RxState.property$ should emit stable reference #6462

* ADD changelog

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

Successfully merging this pull request may close these issues.

2 participants