RootContainers count twice for getAboveTheFoldCount #144
Labels
bug
An issue with the system
Comments
|
Thanks for reporting @lidawang. That's definitely confusing. I think omitting |
gigabo
added a commit
to gigabo/react-server
that referenced
this issue
May 26, 2016
This replaces the `getAboveTheFoldCount()` page method, which is awful: - It's separate from the elements themselves, so even remembering to update it can be a challenge. - If the number of root elements above the fold varies, then duplicative logic may be required to determine the count. - Middleware that inserts elements must remember to _increment_ it. - It has non-obvious behavior when root containers are used above the fold (redfin#144) This closes redfin#161 where @bartkusa proposed this _much_ nicer interface. This is a breaking change. The _default_ was previously to wake up the client controller after the _first_ element. With this change the client controller isn't bootstrapped until either `<TheFold />` is seen, or we're out of elements. So, if unspecified, the entire page is assumed to be above the fold.
gigabo
added a commit
to gigabo/react-server
that referenced
this issue
May 26, 2016
This replaces the `getAboveTheFoldCount()` page method, which is awful: - It's separate from the elements themselves, so even remembering to update it can be a challenge. - If the number of root elements above the fold varies, then duplicative logic may be required to determine the count. - Middleware that inserts elements must remember to _increment_ it. - It has non-obvious behavior when root containers are used above the fold (redfin#144) This closes redfin#161 where @bartkusa proposed this _much_ nicer interface. This is a breaking change. The _default_ was previously to wake up the client controller after the _first_ element. With this change the client controller isn't bootstrapped until either `<TheFold />` is seen, or we're out of elements. So, if unspecified, the entire page is assumed to be above the fold.
davidalber
pushed a commit
to davidalber/react-server
that referenced
this issue
Jul 24, 2016
This replaces the `getAboveTheFoldCount()` page method, which is awful: - It's separate from the elements themselves, so even remembering to update it can be a challenge. - If the number of root elements above the fold varies, then duplicative logic may be required to determine the count. - Middleware that inserts elements must remember to _increment_ it. - It has non-obvious behavior when root containers are used above the fold (redfin#144) This closes redfin#161 where @bartkusa proposed this _much_ nicer interface. This is a breaking change. The _default_ was previously to wake up the client controller after the _first_ element. With this change the client controller isn't bootstrapped until either `<TheFold />` is seen, or we're out of elements. So, if unspecified, the entire page is assumed to be above the fold.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We have a page that has an entire
RootContainerthat's above the fold, but we don't account for it in ourgetAboveTheFoldCountimplementation. While debugging why our waterfall is looking weird (caused by this wrong count number), we noticed that the fix would be to a) take into account theRootContainerin ourgetAboveTheFoldCountimplementation, but also b) increment our count by 2 for each entirely-above-the-foldRootContainer. The latter is extremely confusing and easy to miss - can we either omitRootContainers from being part of the above-the-fold-count (they are elements, but they're container elements that can only containRootElements, which already count for above-the-fold so it seems duplicative), or at least fixing this double-counting thing?The text was updated successfully, but these errors were encountered: