React tracks scroll position internally and forces synchronous layout

[steadicat]

React adds a scroll listener that tracks and caches the scroll position (I assume for some undocumented internal purpose). Additionally, the scroll listener forces a synchronous layout (see screenshot). This seems wasteful, especially considering that the values tracked are never exposed in the API.

What is the scroll listener used for? Can it be removed?

[sophiebits]

ViewportMetrics is used to normalize pageX and pageY on mouse events: SyntheticMouseEvent.js#L63-L73. Perhaps ironically, it was created to prevent synchronous layouts during mouse event handlers.

[gaearon]

I wish these values (as well as the event) were somehow exposed.

[mietek]

👍

[gaearon]

BTW you can obviously have this if you use a Webpack-like bundler:

// get_scroll_y.js

'use strict';

var ReactBrowserEventEmitter = require('react/lib/ReactBrowserEventEmitter'),
    ViewportMetrics = require('react/lib/ViewportMetrics');

ReactBrowserEventEmitter.ensureScrollValueMonitoring();
ViewportMetrics.refreshScrollValues();

function getScrollY() {
  return ViewportMetrics.currentScrollTop;
}

module.exports = getScrollY;

[syranide]

Well, ViewportMetrics is only used for IE8 and possibly other very old browsers (http://www.quirksmode.org/mobile/tableViewport_desktop.html), so we should be able to just disable it for all other browsers and everyone is happy right? (Could still be optimized slightly for IE8... if anyone cares)

Also,
https://github.com/facebook/react/blob/master/src/browser/ui/ReactEventListener.js#L65
https://github.com/facebook/react/blob/master/src/browser/ui/ReactEventListener.js#L165

That makes no sense at all to me, refresh does not take an argument, yet we get the scroll position and provide it as an argument. Instead, refresh gets the scroll position internally and uses that? So we call getUnboundedScrollPosition twice, but only use the result once (also, this should be broken for events inside iframes, for IE8).

Also2,
https://github.com/facebook/react/blob/master/src/browser/eventPlugins/TapEventPlugin.js#L54
I'm pretty sure any browsers that supports touch, also supports pageX.

cc @spicyj

[sophiebits]

I agree that we shouldn't be tracking it in other browsers if we don't need it. I don't know if there's a way to predict whether we'll have .pageX and .pageY before mouse events actually come in.

[syranide]

@spicyj document.createEvent && 'pageX' in document.createEvent('MouseEvent') :)

[syranide]

Another idea being just let resize/scroll set a flag, when a mouse event is created, if the flag is set, update viewport metrics. That way we only update it when needed, but could potentially cause a reflow if there are other events before it (very unlikely though I imagine).

[michalzubkowicz]

This issue causes React based longer lists unusable on mobile.

[lo1tuma]

Is there any progress on this issue? I’m not sure if this is the root-cause for pages rendered with react scrolling extremely slow with firefox mobile on android (e.g. this react website http://isomorphic500.herokuapp.com/featured/highest_rated).

[sophiebits]

@lo1tuma Is that your site? If so, you can try commenting out the logic in refreshScrollValues to see if it makes a difference. If it does, I'm happy to prioritize @syranide's #2271 and try to get it in.

[lo1tuma]

@spicyj No, that is not my site, it is an example react & fluxible page (see: https://github.com/gpbl/isomorphic500), but we have exactly the same issues with scrolling on firefox mobile.
I’ve applied the changes in #2271 manually and it doesn’t solves the problem, so this issue might be unrelated to my problem. I will investigate further.
Thanks for considering to prioritize this. Much appreciated.