- 
                Notifications
    You must be signed in to change notification settings 
- Fork 89
Fix selector resolution with async KiCad footprints #1567
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?
Fix selector resolution with async KiCad footprints #1567
Conversation
| The latest updates on your projects. Learn more about Vercel for GitHub. 
 | 
| @seveibar can you please review! | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too complex, lots of repeated code. Good first attempt
| 
 i will try to simplify it and remove the redundancies (maybe with helper functions) | 
91f28d4    to
    7cb5e53      
    Compare
  
    7cb5e53    to
    8ce02f6      
    Compare
  
    8ce02f6    to
    97c259e      
    Compare
  
    | ) | ||
| }) | ||
|  | ||
| test("trace pcbPath selectors work with regular footprints (baseline)", async () => { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove
| A bit complex still since you're relying on a rerender. Did you check if asyncPhaseDependencies can handle this? Seems to me like it would? | 
| And don't test the baseline, there are hundreds of tests for that. Plus we have a rule one test per file | 
| 
 you're right! but i think asyncPhaseDependencies alone wont work. this only checks a component's own tree. since R1 and trace are siblings, we will have to look from root - the render system itself would need to be modified. | 
| 
 @seveibar what do you suggest? | 
| 
 Hmm interesting, i think it should check the entire tree and look from root. The intent with those dependencies was to make the entire system wait and not let certain parts of the tree move forward. There's an exception for subcircuits which are supposed to be able to independently render but we can ignore that for now | 
| @Aqil-Ahmad nice insights | 
| @Aqil-Ahmad you seem to understand the code pretty well, definitely reach out to me on discord if you're interested in doing more work together: https://tscircuit.com/join | 
40d669b    to
    efb85e9      
    Compare
  
    | 
 Hi @seveibar! can you please review. | 
6cce4ba    to
    8426237      
    Compare
  
    
Fixes tscircuit/tscircuit#1149
/claim tscircuit/tscircuit#1149