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

Automatically set change detection for traits that are being observed #16

Open
krispya opened this issue Nov 3, 2024 · 3 comments
Open

Comments

@krispya
Copy link
Member

krispya commented Nov 3, 2024

Right now change detection is set per updateEach call manually, default off. The proposal is to make change detection be on, off, or auto instead of a boolean, with default being auto. If auto then it would follow these rules:

  • All traits that are observed for changes with onChange are added to a tracking list.
  • If a query result selects for a tracked trait then it automatically enables change detection for that trait only.
  • If the trait is no longer observed, this is automatically switched off.
  • This can be overwritten by setting auto to off.

There was suggestion to make this even more specific, per entity per trait. There is no API for that currently (you can observe all entities for a given trait, not specific entities, and then filter for specific entities) so this should be opened as another issue.

@Ctrlmonster
Copy link
Collaborator

Looks good. Yeah I think entity + trait can be an implementation detail eventually. Since the public listening API requires you to pass an Entity in all cases anyway (unless I'm missing some?). Storing the tuple (trait, entity) in the tracking list should greatly reduce the number of comparisons and thus improve perf, making auto a save option in basically all cases. But since this issue is about the updateEach API, I think it's good as it is 👍

@krispya
Copy link
Member Author

krispya commented Nov 3, 2024

Only the React API makes you pass in an entity. The core API does not (onChange) and neither does tracking queries using the Changed modifier.

@Ctrlmonster
Copy link
Collaborator

I see. Okay let's put that discussion in another issue, since this is pretty much agreed on.

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

No branches or pull requests

2 participants