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
chore: throw an error when trying to subclass BrowserWindow
#41795
base: main
Are you sure you want to change the base?
Conversation
This has come up before (will find ref in the am) and this is intentional, subclassing any of electrons built-in classes is not supported and will cause various issues, mostly over the binding / lifecycle boundaries. I think we even explicitly document somewhere not to subclass our api |
ah gotcha - we should document that more visibly then i'd say as i checked around a bit and couldn't find anything in the ts commit history about intentionality of it |
We should probably make BrowserWindow's constructor throw if it's being instantiated as a subclass. |
BrowserWindow.getAllWindows()
BrowserWindow
@nornagon done - if you have a better idea of where I should throw let me know but trying in |
if (this.constructor.name !== 'BrowserWindow') { | ||
throw new Error('Subclassing BrowserWindow is not supported in Electron'); |
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.
Hm... I think this actually has to happen in C++. By the time _init is called, a bunch of BW initialization has already happened. And I don't think we correctly handle thrown errors in _init—we continue initializing the window regardless. I'm a little surprised the test passes!
Description of Change
Closes #41785.
Explicitly throws an error when users try to subclass BrowserWindow as it causes issues over the binding / lifecycle boundaries.
Also adds some tests.
Checklist
npm test
passesRelease Notes
Notes: Fixed an issue where
BrowserWindow.getAllWindows()
did not return subclasses ofBrowserWindow
.