- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 4.2k
 
Mutate Observer #16183
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
base: main
Are you sure you want to change the base?
Mutate Observer #16183
Conversation
| 
           Welcome, new contributor! Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨  | 
    
5cb7832    to
    61511de      
    Compare
  
    | 
           You added a new example but didn't add metadata for it. Please update the root Cargo.toml file.  | 
    
9ca79c0    to
    66db6df      
    Compare
  
    | 
           You added a new example but didn't add metadata for it. Please update the root Cargo.toml file.  | 
    
66db6df    to
    4d1e9cb      
    Compare
  
    | 
           You added a new example but didn't add metadata for it. Please update the root Cargo.toml file.  | 
    
4d1e9cb    to
    7f1357d      
    Compare
  
    | 
           You added a new example but didn't add metadata for it. Please update the root Cargo.toml file.  | 
    
0e4cdd7    to
    5933212      
    Compare
  
    | 
           You added a new example but didn't add metadata for it. Please update the root Cargo.toml file.  | 
    
f836647    to
    4888eec      
    Compare
  
    | 
           You added a new example but didn't add metadata for it. Please update the root Cargo.toml file.  | 
    
313690e    to
    13bdcf9      
    Compare
  
    | 
           The generated   | 
    
13bdcf9    to
    6d9d23d      
    Compare
  
    | 
           The generated   | 
    
6d9d23d    to
    7d6996d      
    Compare
  
    7d6996d    to
    1175027      
    Compare
  
    1175027    to
    ea62087      
    Compare
  
    ea62087    to
    c98f097      
    Compare
  
    | 
           @alice-i-cecile Hi alice, i thought this pr is ready to review now. What should I do next?  | 
    
| 
           @pyhinox there are some merge conflicts, but I'll go ahead and mark it for review to start getting eyes on it. 🙂  | 
    
| 
           Could you use  CI is failing for a few invalid links in documentation, mostly things that are not in scope for automatic doc links  | 
    
| 
           I thought we had a lint enabled to catch that as well. Good eye.  | 
    
| 
           I think that we can use #16208 to remove the need for this. If certain components can't be mutated, we can do all of our reactive logic in the insertion / removal stage 🎉  | 
    
          
 I don't believe clippy lints apply to documentation sadly. I got bit by that when adding the lints!  | 
    
| 
           @alice-i-cecile how does #16208 remove the need for this? What happens if you want to observe a component changing?  | 
    
          
 I believe the idea was if you want to watch a component, make that component immutable so that the current hooks can capture all state changes. It doesn't cover the case where you want to observe a component which isn't immutable though.  | 
    
          
 Precisely. 
 Yeah. Ultimately, virtually all of the same drawbacks apply to both the OnMutate and the immutable components approaches to this problem: 
 I'd like to see if we can get away with only using a single new concept to achieve these benefits, and then re-evaluate once we have a clearer picture of the final performance and ergonomics. EDIT: on reflection, I think that this is probably still useful, because it allows users to do reactivity things with any arbitrary component, which is really valuable for UI. I suspect that we should have on-mutate observers fill a "not very strict + deferred reactions" niche, while reserving immutable components + hooks for really critical invariants that must be constantly upheld.  | 
    
          
 Yeah I'm starting to want something like this as part of our reactivity solution, over less event driven / poll based approaches (tbd though). But yeah the current unconditional impl should not be merged. If we land this, we can't affect the default Mut path. That is hot / costly enough as it is. The   | 
    
Objective
OnMutateobserver and demonstrate how to use it for UI reactivity #14520 and adds OnMutate observers.Solution
A
EntityChangeswhich stores the change list is added toWorld. WhenMut::set_changedis called, a change record will be added to theEntityChanges.When
World::flushis called,EntityChangeswill be clear and triggerOnMutateevent to each relative entity.Example
Testing
Just test for the base function, I don't know which test i should do.
Query::iterandQuery::get_mut's benchmarkJust treat the
OnMutateas a common observer. Nothing is specific.Mac
Possible Follow-ups
OnMutatehook