ctrlplusb / react-jobs

Asynchronously resolve data for your components, with support for server side rendering.
MIT License
166 stars 34 forks source link

Breaks asyncBootstrap on client side #41

Open SleepWalker opened 7 years ago

SleepWalker commented 7 years ago

If you'll run react-jobs without SSR and will use asyncBootstrap for other needs, that can not be covered by react-jobs at the moment (#39), it will break async bootstrapper from crawling the tree: all the asyncBoostrap() methods won't be called if they are placed after react-job in react tree on client side.

You can check this test out:

// @flow
import React from 'react';
import asyncBootstrapper from 'react-async-bootstrapper';
import { withJob } from 'react-jobs';
import type { Element } from 'react';

class Bootstrap extends React.Component<{
  bootstrap: string => Promise<void>,
  id: string,
  children?: Element<*>,
}> {
  asyncBootstrap(): Promise<boolean> {
    return this.props.bootstrap(`${this.props.id}Bootstrap`).then(() => true);
  }

  render() {
    return this.props.children || null;
  }
}

const HocBootstrap = withJob({
  work: props => props.bootstrap(`${props.id}Work`).then(() => true),
})(Bootstrap);

it('should call asyncBootstrap', () => {
  const bootstrap = jest.fn().mockReturnValue(Promise.resolve());

  const App = (
    <div>
      <HocBootstrap bootstrap={bootstrap} id="hoc">
        <Bootstrap bootstrap={bootstrap} id="innerHoc" />
      </HocBootstrap>
      <Bootstrap bootstrap={bootstrap} id="noHoc">
        <Bootstrap bootstrap={bootstrap} id="innerNoHoc" />
      </Bootstrap>
    </div>
  );

  return asyncBootstrapper(App).then(() => {
    expect(bootstrap.mock.calls).toEqual([
      ['hocWork'],
      ['noHocBootstrap'],
      ['innerNoHocBootstrap'],
      ['hocBootstrap'],
      ['innerHocBootstrap'],
    ]);
  });
});

You'll receive the following output:

 FAIL  app/__tests__/asyncBootstrap.test.js
  ✕ should call asyncBootstrap (6ms)

  ● should call asyncBootstrap

    expect(received).toEqual(expected)

    Expected value to equal:
      [["hocWork"], ["noHocBootstrap"], ["innerNoHocBootstrap"], ["hocBootstrap"], ["innerHocBootstrap"]]
    Received:
      [["hocWork"], ["noHocBootstrap"], ["innerNoHocBootstrap"]]

    Difference:

    - Expected
    + Received

    @@ -6,12 +6,6 @@
          "noHocBootstrap",
        ],
        Array [
          "innerNoHocBootstrap",
        ],
    -   Array [
    -     "hocBootstrap",
    -   ],
    -   Array [
    -     "innerHocBootstrap",
    -   ],
      ]

I think, that we should always resolve asyncBootstrap methods, even if env === 'browser' to provide consistent behavior. If asyncBootstrap was called, react-jobs must resolve work, so that it will be completed and the render can be continued to the next component.

@ctrlplusb what do you think? I really need to solve this and #39. I can create PR if you'll accept the changes