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

Create custom eslint rule for warning about layout thrashing #41

Open
kaisermann opened this issue Jan 31, 2020 · 1 comment
Open

Create custom eslint rule for warning about layout thrashing #41

kaisermann opened this issue Jan 31, 2020 · 1 comment
Assignees
Labels
discussion Something isn't working todo

Comments

@kaisermann
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Layout thrashing is when someone modifies or accesses (!) some layout properties of a DOM node that triggers the browser layout calculations. Something as simple as

const top = element.offsetTop

can generate more work for the browser. This kind of properties are commonly used in scroll/resize/mouse events and can lead to janky behavior.

Describe the solution you'd like

We can create a custom rule to check for member expressions with certain identifiers, such as offsetLeft, offsetTop, offsetWidth, offsetHeight, offsetParent, clientLeft, clientTop, clientWidth, clientHeight, getClientRects, getBoundingClientRect and much more. See this GIST for more examples: https://gist.github.com/paulirish/5d52fb081b3570c81e3a. The rule would be turned on as a warning and not an error and developers could choose to ignore it if they know what they're doing.

The first problem with this approach is that it can lead to some false positives:

const a = { 
  offsetTop: 20
}
a.offsetTop // would trigger a warning

I'm okay with these kind of false positives, since it's easy to add a ignore line. However, we could also try checking how the @typescript-eslint uses type information to only check for variables that are instance of HTMLElement or something like that.

I think the benefits of warning people about these kind of performance issues is way greater than expecting people without much experience on how the browser works to write performant code.

@kaisermann kaisermann self-assigned this Jan 31, 2020
@kaisermann kaisermann added discussion Something isn't working todo labels Jan 31, 2020
@gtkatakura
Copy link
Contributor

I think we need to configure a rule for that, even if we have false positives as you say.

Just one real case that I recently identified on a screen on Indeva system to show that the impacts of this are too costly (just caching access to the clientWidth, in this case, increased the performance from 4.2s to 60ms)

Screenshot from 2020-08-09 15-28-40

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Something isn't working todo
Projects
None yet
Development

No branches or pull requests

2 participants