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

fix: Various compounding memory leaks in editor #5950

Merged
merged 3 commits into from Oct 6, 2023

Conversation

tommoor
Copy link
Member

@tommoor tommoor commented Oct 5, 2023

closes #5939
closes #5929

@@ -171,7 +171,7 @@
"prosemirror-state": "^1.4.3",
"prosemirror-tables": "^1.3.2",
"prosemirror-transform": "^1.7.3",
"prosemirror-view": "^1.31.3",
"prosemirror-view": "^1.32.0",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const containerBounds = useComponentSize(
document.body.querySelector("#full-width-container")
);
const documentBounds = props.view.dom.getBoundingClientRect();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use cached bounds for document, and one less ResizeObserver

@@ -14,48 +14,44 @@ const defaultRect = {
export default function useComponentSize(
element: HTMLElement | null
): DOMRect | typeof defaultRect {
const [size, setSize] = useState(element?.getBoundingClientRect());
const [size, setSize] = useState(() => element?.getBoundingClientRect());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't calculate getBoundingClientRect on ever render

);
});
const sizeObserver = new ResizeObserver(() => {
element?.dispatchEvent(new CustomEvent("resize"));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Custom event allows handling to be in one handleResize function

@@ -302,6 +302,7 @@ export class Editor extends React.PureComponent<

public componentWillUnmount(): void {
window.removeEventListener("theme-changed", this.dispatchThemeChanged);
this.view.destroy();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memory leak, not calling destroy on NodeView s on unmount

@tommoor tommoor merged commit eb71a8f into main Oct 6, 2023
11 checks passed
@tommoor tommoor deleted the tom/5939-complete-crash-of-the-application branch October 6, 2023 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Complete crash of the application Full width images don't resize correctly with sidebar events
1 participant