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.

Sign up for GitHub

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

Jump to bottom

Added note in docs for change listeners #5749

Merged
merged 3 commits into from
Apr 20, 2023
Merged

Added note in docs for change listeners #5749

merged 3 commits into from
Apr 20, 2023

Conversation

papafe
Copy link
Contributor

@papafe papafe commented Apr 19, 2023

This closes #927

@cla-bot cla-bot bot added the cla: yes label Apr 19, 2023
elle-j
elle-j reviewed Apr 19, 2023
View reviewed changes
packages/realm/src/Collection.ts Outdated Show resolved Hide resolved
packages/realm/src/Object.ts Outdated Show resolved Hide resolved
kraenhansen
kraenhansen approved these changes Apr 19, 2023
View reviewed changes
Copy link
Member

@kraenhansen kraenhansen left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I believe this applies for the Realm#addListener too.
Perhaps it's worth mentioning why this is .. specifically that adding a listener is an asynchronous operation and the first event is fired for the caller to know when the listener has been added. (at least that was the explanation @tgoyne once gave to me).

@papafe
Copy link
Contributor Author

papafe commented Apr 20, 2023

I believe this applies for the Realm#addListener too. Perhaps it's worth mentioning why this is .. specifically that adding a listener is an asynchronous operation and the first event is fired for the caller to know when the listener has been added. (at least that was the explanation @tgoyne once gave to me).

I agree with adding the explanation. I've tried running some tests using Realm.addListener, and it seems it doesn't apply to that.

@papafe papafe merged commit f8e0cc9 into main Apr 20, 2023
@papafe papafe deleted the fp/listener-doc branch April 20, 2023 13:39
papafe added a commit that referenced this pull request Apr 21, 2023
@papafe
Merge branch 'main' into fp/core-logging
0044a20 
* main:
  Added note in docs for change listeners (#5749)
  Update CHANGELOG entry (#5751)
  Use `SchemaParseError` for invalid schemas (#5722)
  Improving Electron tests (#5738)
  Support SSL Sync Configuration (#5507)

# Conflicts:
#	CHANGELOG.md
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@elle-j elle-j elle-j left review comments

@kraenhansen kraenhansen kraenhansen approved these changes

@kneth kneth kneth approved these changes

Assignees

@papafe papafe

Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

addListener notification is empty (documentation only)
4 participants
@papafe @kneth @kraenhansen @elle-j