Skip to content

misc: deduplicate all the dom helpers#15960

Merged
connorjclark merged 2 commits intomainfrom
one-find
Apr 17, 2024
Merged

misc: deduplicate all the dom helpers#15960
connorjclark merged 2 commits intomainfrom
one-find

Conversation

@connorjclark
Copy link
Copy Markdown
Collaborator

fixes #11992

This reduces 4 instances of find to 1. I also adopted treemap's default second param to the canonical instance in DOM.

@connorjclark connorjclark requested a review from a team as a code owner April 17, 2024 19:34
@connorjclark connorjclark requested review from adamraine and removed request for a team April 17, 2024 19:34
import {renderFlowReport} from '../../../flow-report/api';

// @ts-expect-error Legacy use of report renderer
const dom = new DOM(document);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not just add document.documentElement to get rid of the type error?

Copy link
Copy Markdown
Collaborator Author

@connorjclark connorjclark Apr 17, 2024

Choose a reason for hiding this comment

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

because its more work than just that

image

so i'll do it in a separate PR

#13821

@brendankenny
Copy link
Copy Markdown
Contributor

Could it be a standalone function that these files could import and that dom.js could use as well? Feels weird to pull in and instantiate all of DOM for this one function that's not static only for (extremely) historical reasons.

@connorjclark
Copy link
Copy Markdown
Collaborator Author

That's a reasonable further refactor. Note that treemap uses createElement/createChildOf too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"find element" code is duplicated in many places

4 participants