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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds requestIdleCallback to vdom/component #409

Open
wants to merge 2 commits into
base: beta
from

Conversation

Projects
@paullewis

paullewis commented Nov 18, 2016

So from our offline conversation, I've been working on adding requestIdleCallback to Component. This is a straw man approach, so I'm not going to be all hurt if you're like "ick, no, don't do THAT!" or anything 馃槃

What this does is wraps the renderComponent function inners in requestIdleCallback if the component in question is set to render as async:

class MyComponent extends Component {
  get renderAsync () {
   return true;
  }

  render () {
    // usual gubbins...
  }
}

If the component is pending reconciliation and a second call to renderComponent is made for it, it cancels the first and replaces it with the most recent.

When faced with a decent number of components to boot from SSR, I'm seeing the main thread blocking time on my Nexus 5X go from ~1 second:

before

All the way down to ~160ms:

after

Obviously this could be a breaking change, though given it's an opt-in behaviour I hope it wouldn't really sting anyone!

I mean, who doesn't enjoy a 6x reduction in blocking main thread time? 馃帀 馃帀 馃帀 馃帀

paullewis added some commits Nov 18, 2016

@addyosmani

This comment has been minimized.

addyosmani commented Nov 18, 2016

6x is such a nice win. Great work, @paullewis 馃帀 Shipping it as an opt-in (at least for now) should give folks a chance to try it out for a while in the wild and let you know if anything breaks.

component.componentWillReceiveProps(props, context);
}
const cancelIdleCallback =
requestIdleCallback !== 'null' ?

This comment has been minimized.

@tony19

tony19 Nov 18, 2016

'null'? Did you mean null?

requestIdleCallback !== null

This comment has been minimized.

@developit

developit Nov 19, 2016

Owner

whoops!

This comment has been minimized.

@paullewis

paullewis Nov 20, 2016

Nah, I didn't. I did a typeof test earlier on, and that carried through. Awwwwkward.

This comment has been minimized.

@makingoff

makingoff Nov 22, 2016

@paullewis you did return window.requestIdleCallback to const requestIdleCallback if typeof window !== 'undefined' && typeof window.requestIdleCallback !== 'undefined'. So, now requestIdleCallback contains a function or null. Not a typeof from earlier test.

This comment has been minimized.

@aweary

aweary Dec 2, 2016

@makingoff is right, requestIdleCallback will be null not 'null' when window.requestIdleCallback is not defined, so cancelIdleCallback will be undefined instead of null in that case.

screen shot 2016-12-02 at 3 47 44 pm

It doesn't actually cause an error since cancelIdleCallback is never called when requestIdleCallback is not defined, but still worth fixing 馃槃

@developit

This comment has been minimized.

Owner

developit commented Nov 28, 2016

Looking into some breakages from this, sorry for the merge wait!

@ainth

This comment has been minimized.

ainth commented Dec 12, 2016

I like the idea but wonder if this should just be a component and not part of the framework. Roughly:

Instead of:

  get renderAsync () {
   return true;
  }

it would be:

render() {
  return (
    <AsyncRenderer>
      ...
    </AsyncRenderer>
  );
}

Rough implementation:

class AsyncRenderer extends Component {
  constructor(props) {
    super(props);
    this.state = { readyToRender: false };
  }

  componentWillMount() {
    requestIdleCallback(() => {
      this.setState({ readyToRender: true });
    })
  }

  render() {
    if (this.state.readyToRender) {
      return this.props.children;
    }
    return (<span />);
  }
}

(this would need a lot more to work in real life but hopefully conveys the idea). This feels more idiomatic and wouldn't require breaking changes or an expansion of the API.

@matthewp

This comment has been minimized.

Contributor

matthewp commented Dec 12, 2016

One thing I like about @ainth's idea is that it allows for the async rendering to be contextual. I could see having different variations of AsyncRenderer for different strategies.

For example, I could see an InViewRenderer that waits until the component is within some threshold of being in the viewport before rendering. How would you do stuff like that using the renderAsync approach?

@developit

This comment has been minimized.

Owner

developit commented Dec 12, 2016

We have a <RenderOnVisible /> that does what you described @matthewp:

example

Not compatible with SSR though, since it uses getBoundingClientRect(). That said, for lazy-loaded items it might be better to skip SSR.

I think that's the most interesting part of Paul's implementation here: you can render on the server, but then just slowly hydrate as needed on the client. I think this is a really interesting space.

@developit

This comment has been minimized.

Owner

developit commented Dec 12, 2016

Also: for people who are as interested as I am with this - here's a really interesting demo of Paul's amended version of Preact asynchronously rendering a pythagoras tree. You can see the effect of the fully async rendering as the tree depth grows beyond what your CPU can render in in a single pass - it starts to render progressively based on available time.

d52c0411766954026ad68af32d1ae253

Demo: http://jsfiddle.net/developit/n733adf9/

@marvinhagemeister

This comment has been minimized.

Collaborator

marvinhagemeister commented Dec 12, 2016

@developit That tree demo is mesmerising! I love where this is going 馃憤

@sebmarkbage

This comment has been minimized.

sebmarkbage commented Dec 13, 2016

This demo looks very similar to the Sierpinski Triangle Demo that we've been using to test React Fiber's scheduling. The difference there is that we can hold the rendering to the screen until everything is complete.

We do have a mode that demos the progressive work it does but we only use that to demo the work that is done behind the scenes - not user visible.

In most cases you don't actually want to show the progression of too many incomplete states to the user. For example on Facebook we've actively been trying to get rid of that since the load experience is jarring when there are too many small visible updates to the screen. So instead of showing them immediately we delay them for later. In the mean time we can still commit smaller updates elsewhere if they're higher priority for responsiveness so nothing blocks.

@developit

This comment has been minimized.

Owner

developit commented Dec 13, 2016

@sebmarkbage I believe this demo code was actually @Swizec's that I punked to test out this rendering mode. I do have a preact version of that triangle demo floating around somewhere though, it shows how the current async rendering works prior to Paul's PR here - not supporting async sibling rendering.

Your point about perceived lag and meaningful scheduling is the thing that's making me think this PR through perhaps a little more thoroughly than I ought to. I don't think there's a one-size-fits-all scheduler, so I want to provide a clean extension point so that people can build their own schedulers and publish them. There are many use-cases, so I foresee many scheduling techniques to match.

@sebmarkbage

This comment has been minimized.

sebmarkbage commented Dec 13, 2016

@developit Certain scheduling techniques are anti-modular. Those are the ones that need built-in support. Anything else can be user space though.

@pemrouz

This comment has been minimized.

pemrouz commented Dec 14, 2016

I don't think there's a one-size-fits-all scheduler, so I want to provide a clean extension point so that people can build their own schedulers and publish them. There are many use-cases, so I foresee many scheduling techniques to match.

This is very important: How would this for example work with the microtask or rAF batching? It would be very hard for users to compose/control strategies with a true/false API or some other config.

Instead, I think it would be better for the framework to expose primitives to the user, for example like pauseMeIfNeccessary that can be naturally awaited:

async render(node, data){
  const { pause } = framework
  // render some things
  await pause()
  // render some things
  await pause()
  // render some things
}

Just awaiting on a noop would push the component to the back of the render queue. You could have another one (deopt?) that would use rIC that would essentially make it low priority by only continuing rendering when other components have rendered (even if they joined the queue after the component was paused). You could potentially allow passing a value to indicate different priority lanes for rendering components. And a timeout. You could pause until visible, but perhaps first just fixing width/height though so the container has the right scrollbar length etc etc. Furthermore, these common cases could also be used by render middleware by decorating the component rather than from inside. This extension point allows more innovation to take place outside the framework core.

The key change to allow progressive rendering is that the render function needs to be async. This allows the component to stream/pipe/flush partial updates to the DOM early, rather than having to do it all at once in the final return.

The main objection I've heard against this is that it's no longer "pure". What's important with that is that components are still declarative and a function of data, it just doesn't have to resolve immediately but can over time.

The overall render process is fundamentally async. In most cases you have to wait for data to load. With the above, you can just do await import(dep) too.

Forcing the tree of all components to be sync causes a lot of awkwardness atm for UI developers: it either pushes developers to try ideologically load everything at the root (doesn't scale), or splitting into an arbitrary async phase and sync phase with more lifecycle (i.e. getInitialProps, which is again only called at top-level), or causes performance problems (like having to double render) or a huge spike in complexity with other third-party solutions (sagas, thunks, etc).

Related:
[1] Need to manually re-call getInitialProps of sub-components
[2] Support asynchronous server rendering (waiting for data before rendering)
[3] proposal: await dependencies inside component function

@developit

This comment has been minimized.

Owner

developit commented Dec 14, 2016

Wouldn't a generator be more appropriate than an async function for modelling this?

For what it's worth, making render asynchronous does not solve Paul's issue on its own. If you render 500 components in a common parent (as this demo does), each is not sufficiently aware of the other or its place in the try that it could determine an appropriate priority for itself. Centralizing that decision into a hook makes this possible, since all component render requests flow through a single point that can use as much state as it needs to implement an appropriate algorithm for prioritizing updates.

@swernerx

This comment has been minimized.

swernerx commented Dec 16, 2016

@developit what is this <RenderOnVisible /> thingy? Do you have a pointer to source / docs for this... sounds super useful!

@luisvinicius167

This comment has been minimized.

Contributor

luisvinicius167 commented Dec 16, 2016

@developit

This comment has been minimized.

Owner

developit commented Dec 16, 2016

Yeah @luisvinicius167's reference is the closest I have publicly available. The other one is internal. I'll ask about open-sourcing it

@jide

This comment has been minimized.

jide commented Jan 19, 2017

Wow ! You may also want to have a look at https://github.com/Synchronized-TV/react-async-child (the react-router 4 example there is interesting)

@developit

This comment has been minimized.

Owner

developit commented Jan 19, 2017

@jide That's similar to what I had originally suggested (AsyncComponent). However, Paul pointed out that it breaks when using Server-Side Rendering. The statically rendered HTML element children of the lazily-rendered component get removed. The hope here is to work around that by not only lazily rendering children, but the component itself.

@jide

This comment has been minimized.

jide commented Jan 19, 2017

Ah ok, I better understand the issue !

@kurtextrem

This comment has been minimized.

kurtextrem commented Aug 4, 2017

@developit Any new plans yet regarding this PR? :)

@kurtextrem

This comment has been minimized.

kurtextrem commented Mar 7, 2018

By the way, as React Suspense is on the way. Might this PR be the beginning of Preact Suspense? 馃

@cristianbote

This comment has been minimized.

cristianbote commented Oct 23, 2018

Hey is this still on the table? 馃檪 Are you guys considering it?

@paullewis

This comment has been minimized.

paullewis commented Oct 23, 2018

Hey is this still on the table? 馃檪 Are you guys considering it?

I'm always considering it... /me stares at @developit 馃榿

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