- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 3.6k
Add unmatched push/pop warning #8213
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: dev-2.0
Are you sure you want to change the base?
Conversation
| Hii @davepagurek can you review this and let me know if anything needs to change thank you!! | 
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.
Hi, in the screenshot when there's a loop of 100 push() calls without pop() calls, the friendly error message is showing something around 200. Is that expected? or it's something like showing the unmatched stack depth?
| Hii @perminder-17 i'm glad you mentioned this and yes, that's expected because the warning triggers when depth exceeds the threshold (100), and by the time it shows the warning (after frame 2), the depth is already 200! | 
        
          
                src/core/main.js
              
                Outdated
          
        
      | this._frameRate = 1000.0 / this.deltaTime; | ||
| await this.redraw(); | ||
| // Check for unmatched push/pop calls (only warn once) | ||
| const PUSH_POP_WARNING_THRESHOLD = 100; | 
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.
Do you think the threshold needs to be 100, or should it just be > 0?
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.
i thought it will give some flexibility and only warns when it becomes a real problem, there is no downside of using >0. i will update it thank you @davepagurek
Resolves #8209
Changes:
added a Friendly Error System (FES) warning to detect unmatched push() and pop() calls that can cause performance degradation and browser freezing.
What was added:
Screenshots of the change:
PR Checklist
npm run lintpasses