Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Add shape model functionality to React components #2320

Closed

Conversation

trueadm
Copy link
Contributor

@trueadm trueadm commented Jul 25, 2018

Release notes: React components can have their props modelled via __optimizeReactComponentTree

This extends the current shape modelling functionality added for InstantRender (great work by @hotsnr ) and provides the same powers to the React compiler. An example of usage:

var React = require("React");

function App(props) {
  return <div>
    <h1>{props.header.toString()}</h1>
    <ul>
      {
        props.items && props.items.map(item =>
          <li key={item.id}>{item.title.toString()}</li>
        )
      }
    </ul>
  </div>;
}

App.getTrials = function(renderer, Root) {
  var items = [{ id: 0, title: "Item 1" }, { id: 1, title: "Item 2" }, { id: 2, title: "Item 3" }];
  renderer.update(<Root items={items} header={"Hello world!"} />);
  return [["render simple with props model", renderer.toJSON()]];
};

if (this.__optimizeReactComponentTree) {

  let universe = {
    Item: {
      kind: "object",
      jsType: "object",
      properties: {
        id: {
          shape: {
            kind: "scalar",
            jsType: "integral",
          },
          optional: false,
        },
        title: {
          shape: {
            kind: "scalar",
            jsType: "string",
          },
          optional: false,
        },
      },
    },
    Props: {
      kind: "object",
      jsType: "object",
      properties: {
        header: {
          shape: {
            kind: "scalar",
            jsType: "string",
          },
          optional: false,
        },
        items: {
          shape: {
            kind: "array",
            jsType: "array",
            elementShape: {
              shape: {
                kind: "link",
                shapeName: "Item",
              },
              optional: false,
            },
          },
          optional: true,
        },
      },
    },
  };

  let appModel = {
    component: {
      props: "Props",
    },
    universe,
  };

  __optimizeReactComponentTree(App, {
    model: JSON.stringify(appModel),
  });
}

module.exports = App;

@trueadm trueadm requested a review from NTillmann July 25, 2018 09:23
} else if (typeof value === "string") {
// string options
if (key === "model") {
modelString = value;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably want to do some sort of input validation here to make sure that the string parses and won't throw an unhandled error later. You can probably just grab the logic from __optimize.

Copy link
Contributor

@calebmer calebmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn’t know that this was how Instant Render built models. Have we considered using __abstract to build up models? You can then use JS for sharing and you can build abstractions on top of __abstract calls. Here is how I imagine __optimize working.

var optimizedFunction = (a, c) => __optimize(
  (a, b, c) => functionToOptimize(a, b, c),
  __abstract('number', 'a'),
  42,
  __abstract('string', 'c'),
);

Instead of replacing functionToOptimize in the output, Prepack would create a new optimizedFunction and would output functionToOptimize as a residual function if it were still needed.

This approach has the advantage of specializing a function with multiple kinds of arguments.

var optimizedFunction1 = (x) => __optimize(
  (a, b) => functionToOptimize(a, b),
  true,
  __abstract('string', 'x'),
);

var optimizedFunction2 = (x) => __optimize(
  (a, b) => functionToOptimize(a, b),
  false,
  __abstract('string', 'x'),
);

For instance, here you can specialize a function twice and provide both concrete and abstract arguments. This same strategy could also be used for __optimizeReactComponentTree.

This strategy is better for the general case of optimizing functions/components, but this path of stringified models definitely seems better for now. (Since it already exists.)

@@ -73,6 +73,11 @@ export type ArgModel = {
arguments: { [string]: string },
};

export type ComponentModel = {
universe: ShapeUniverse,
component: { [string]: string },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason this isn’t this?

component: {props: string},

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eventually I wanted to maybe add context and state, so left it open. But I'll change this.

@trueadm
Copy link
Contributor Author

trueadm commented Jul 25, 2018

@calebmer That was one of the ideas I had too, but it was more involved and I wanted to try and re-use existing logic where possible for now to see if type data gave much value. We can always re-factor it to that approach later though.

@hotsnr
Copy link

hotsnr commented Jul 25, 2018

@calebmer @trueadm The main point of creating this modeling was that mechanism under __abstractOrNull is not working properly (see #2158). If your model always has optional: false then there it may not have such a value for you.

@trueadm
Copy link
Contributor Author

trueadm commented Jul 25, 2018

@hotsnr Unfortunately, most things in our case come from GraphQL and are also optional too.

@calebmer
Copy link
Contributor

Also, if we had a a mechanism like the one I described above we might be able to implement the React Compiler first render optimization in user code:

const firstRender = (props) => __optimize(
  (props) => React.renderOnce(React.createElement(MyComponent, props)),
  __abstract(/* insert model here */, props),
);

(React.renderOnce might also be React.renderToString?)

@trueadm
Copy link
Contributor Author

trueadm commented Jul 25, 2018

@calebmer I don’t think we ever want to do that. We need to keep the model in Prepack because of all the complex things we need to do. For example we snapshot props and ReactElement objects do they are temporal and have bunch of equivalence optimization’s etc, not to mention cloning of them when we evaluate them. We should speak about it on VC again some time :)

Copy link

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@trueadm is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants