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.

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

[v2] Node's constructor called from editor.registerNodeType #309

Open
yojeek opened this issue Jul 20, 2023 · 1 comment
Open

[v2] Node's constructor called from editor.registerNodeType #309

yojeek opened this issue Jul 20, 2023 · 1 comment
Labels
documentation Issues about the docs v2 Related to Baklava v2

Comments

@yojeek
Copy link
Contributor

yojeek commented Jul 20, 2023

Calling editor.registerNodeType(Node) will instantiate Node's class. I find this behaviour quite unexpected, especially because onDestroy method which I suppose must be used for cleanup of Node's dependencies wasn't called.

Is it really necessary to instantiate the class? Can't "category" and "title" be static properties?

public registerNodeType(type: AbstractNodeConstructor, options?: IRegisterNodeTypeOptions): void {
        if (this.events.beforeRegisterNodeType.emit({ type, options }).prevented) {
            return;
        }
        const nodeInstance = new type();
        this._nodeTypes.set(nodeInstance.type, {
            type,
            category: options?.category ?? "default",
            title: options?.title ?? nodeInstance.title,
        });
        this.events.registerNodeType.emit({ type, options });
    }

At least it should be documented because anything what is created inside constructor and not cleaned may potentially live forever (in my case it was websocket client polling for connections).

@newcat
Copy link
Owner

newcat commented Jul 22, 2023

Unfortunately, abstract static properties are currently not supported in TypeScript (microsoft/TypeScript#34516). I could use static properties but this would not be enforced by the type system, so I opted for this approach instead. But I'll add some more documentation to emphasize the fact that the constructor is called during node registration.

@newcat newcat added documentation Issues about the docs v2 Related to Baklava v2 labels Jul 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Issues about the docs v2 Related to Baklava v2
Projects
None yet
Development

No branches or pull requests

2 participants