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

Object ownership: Invalid mutation for Linked class when using set/getContext #15532

Open
henrykrinkle01 opened this issue Mar 17, 2025 · 1 comment · May be fixed by #15678
Open

Object ownership: Invalid mutation for Linked class when using set/getContext #15532

henrykrinkle01 opened this issue Mar 17, 2025 · 1 comment · May be fixed by #15678

Comments

@henrykrinkle01
Copy link
Contributor

Describe the bug

This is a follow-up of the issue #15164, which has been fixed by the PR #15166.
I have a Linked class and a linked function which does the same thing.
Both objects created by the class and the function are passed to a child component using the context API.
In the child component, mutating the object created by the class produces an ownership_invalid_mutation warning while the function version is fine.

Reproduction

https://svelte.dev/playground/ccf69c3f620a402eadb39b0715a5976b?version=5.23.0

Logs



System Info


N/A


Severity


annoyance

      		
    

      
  





        



            

              
            

        

      


      
    








      

    



      

  
      



  

  @dummdidumm





  


    

      

  

    

        
    
    

  


      
          
  
      
            Copy link

  

      
    

  


  

      

  
  Member


      

  


  

    

  





      


        


  
    

      
          
The problem is that we're not adding private state/derived declarations to the ADD_OWNERS symbol method

      		
    

  





        


            

            

              
            

        

      


      

            
  

    
  
    
    

  

  



    












      

  
          


  
    
  
  

    
  dummdidumm 

      added a commit
      that referenced
      this issue

    
      Apr 3, 2025
    
      
        
      
    

      


  

    

      


  

      
        @dummdidumm
  





      

        
          fix: rework binding ownership validation
        

          
            
          

      


      

          

    
    
    

  

      


      

      


      
      

        
          88a83d6
        
      

    

  

    

      
Previously we were doing stack-based retrieval of the owner, which while catching more cases was costly (performance-wise) and prone to errors (as shown by many issues over the months).

This drastically simplifies the ownership validation - we now only do simple static analysis to check which props are mutated and wrap them with runtime checks to see if a binding was established.

Besides making the implementation simpler and more performant, this also follows an insight we had over the months: Most people don't really know what to do with this warning when it's shown beyond very simple cases. Either it's not actionable because they don't really know how to fix it or they question if they should at all (in some cases rightfully so). Now that the warning is only shown in simple and easy-to-reason-about cases, it has a much better signal-to-noise-ratio and will hopefully guide people in the right direction early on (learn from the obvious cases to not write spaghetti code in more complex cases).

closes #15532
closes #15210
closes #14893
closes #13607
closes #13139
closes #11861

    




    

  




  

  

    
  


  

      

    @dummdidumm
dummdidumm



    linked a pull request
    
      Apr 3, 2025
    

    that will
    close
    this issue





  


  

      
   Open

  





  
  

      
        
      
      

  
    
    
  


    6 tasks
  





  













  
  

    

      

  





  




  
  

              

  
    Sign up for free
    to join this conversation on GitHub.
    Already have an account?
    Sign in to comment


  



  






  
                  




    

  


      
  

    Assignees
  



        

    No one assigned







      


  


  

    Labels
  



    

    None yet







      

  

    

        

    Projects
  


        




    None yet




  



      


  

    
  

    Milestone
  


      No milestone





    
      
          


  

    

      
        

          
    

    Development
  




            
  
Successfully merging a pull request may close this issue.





    


  
          
      fix: rework binding ownership validation

    
      sveltejs/svelte
    





      
    

  




      

    

  
      

  

    

      2 participants
    

    

        
          @dummdidumm 
        
          @henrykrinkle01