Add support for React Hooks under a new React module namespace

[rickyvetter]

This PR provides APIs for all hooks as well as element creation APIs, core components (Suspense, Fragment) and other top-level React APIs. It moves Router out ReasonReact.re since it can be used in with either type of components. It adds a ppx (to be moved into BuckleScript). Finally, it provides a module called ReasonReactCompat, which has bindings that can be used for both forwards and backwards compatibility (these functions will be very familiar to anyone who has worked with JS interop in the current API).

This new way of writing components comes with a number of benefits.

First - components written this way are valid ReactJS components and can be used in interop scenarios without additional binding.
Second - it provides a way forward to support newer React features like Suspense. It will also make writing components for concurrent mode much easier.

Usage

/* writing a component: */
[@react.component]
let make = (
  ~prop1,
  ~prop2,
  ~children,
) => {
  <Foo prop1 prop2> children </Foo>
};

/* unwrapping the ppx, this looks something like: */
[@bs.obj]
external makeProps: (~key: option('key)=?, ~prop1: 'prop1, ~prop2: 'prop2, ~children: 'children, unit) => {.
  "prop1": 'prop1,
  "prop2": 'prop2,
  "children": 'children,
} = "";

let make = (Props) => {
  let prop1 = Props##prop1;
  let prop2 = Props##prop2;
  let children = Props##children;
  <Foo prop1 prop2> children </Foo>
};

/* final JS output might look like: (note this is a valid React component!) */
function NameOfModuleWeAreIn(Props) => {
  var prop1 = Props.prop1;
  var prop2 = Props.prop2;
  var children = Props.children;
  return React.createElement(Foo.make, {prop1, prop2, children});
};
let make = NameOfModuleWeAreIn;

/* external */
[@react.component][@bs.module]
external make: (~prop1: int, ~prop2: string, ~children: React.element) => React.element = "./Foo";

/* interface */
[@react.component]
let make: (~prop1: int, ~prop2: string, ~children: React.element) => React.element;

Things to Note

• children - in ReactJS the type of children is quite complex. Originally this implementation wrapped multiple static children (<A> <B/> <C/> </A>) with a fragment, but now it wraps with an array and includes null in the children object. This only applies for non-primitive components which accept >1 static children. For all other components behavior is identical to the JS variant of JSX.

TODOs

• Documentation - lots can be deprecated, but use of the JSX and the [@react.component] tag is something we should be careful to document well
• Locations - this PPX is relatively complex and one of the trickiest parts is making sure all of the code points to the correct locations. This is very important for accurate error messages and type information. If you notice any issues please report

[baransu]

As far as I’m concerned (please correct me if I’m wrong), right now there is now way using JSX to pass data-* attributes like data-testid to built in components. Is there any chance something may change in that matter?

[rickyvetter]

@baransu please see #230 for the tracking issue and a workaround - unfortunately it's not possible to add support for these attributes directly because they are dynamic. If facebook/react#1259 was adopted then we could support them first-class. This is true in both the current record-based api as well as this new hooks api.

[bordoley]

Ricky and I discussed this in private, but one proposal I wanted to make would be to try to avoid special casing react dom in the PPX. The current design prevents use cases such as defining a single module with multiple react components that depend upon each other. For instance,

[@react.component]
let componentA = (~prop1, ()) => <div/>;

[@react.component]
let componentB = (~prop1, ()) => <componentA/>;

is not supported because all lower case names are required to be string DOM components. Granted this can be addressed by adding a wrapper module around componentA, but this is extra boiler point that it would be nice to avoid.

Instead we could support both use cases by exporting the dom string names from ReactDOM. One issue right now is that strings are not inlined by bucklescript, so the bindings would introduce the cost of shipping a compiled form of ReactDOM. Perhaps we can work with bucklescript to support some external string type that is always inline at the use point?

[phated]

Bringing over from Discord as per Ricky

I co-locate pretty much everything within a component, including the React component logic. Up until now, the requirement of ReasonReact on a make function made for really weird APIs. Take for example:

module Person = {
  type t = {
    name: string,
    age: int,
  };

  // Canonical Reason/OCaml would dictate this be `make`
  let create = (~name, ~age) => {name, age};

  let component = ReasonReact.statelessComponent("Person");

  let make = (~person, _children) => {
    ...component,
    render: _self =>
      <div>
        <span> {ReasonReact.string(person.name)} </span>
        <span> {ReasonReact.string(string_of_int(person.age))} </span>
      </div>,
  };
};

Usage would look like:

let me = Person.create(~name="Blaine", ~age=99);

let rendered = <Person person=me />

The new addition of the [@react.component] attribute allows us to annotate any method as a component, which is AWESOME(!!!!). So our code can change to:

module Person = {
  type t = {
    name: string,
    age: int,
  };

  let make = (~name, ~age) => {name, age};

  [@react.component]
  let component = (~person) =>
    <div>
      <span> {ReasonReact.string(person.name)} </span>
      <span> {ReasonReact.string(string_of_int(person.age))} </span>
    </div>;
};

But that makes the usage less ergonomic (if you aren't using the implicit make function)

let me = Person.create(~name="Blaine", ~age=99);

let rendered = <Person.component person=me />

Since we are adding an attribute to all our React components, why can't we look for an attribute that indicates the method to compile a top-level usage instead of implying make? Some example APIs I was thinking about:

[@react.component.default]
let component = (~person) => <div />;

[@react.component] [@react.default]
let component = (~person) => <div />;

[@react.component default]
let component = (~person) => <div />;

This would keep the ergonomics of the make method without stomping on a common name used in OCaml/Reason and without typing us to the "default" export of ecmascript. The "default" token could be renamed to "main" or something, the name doesn't matter to me as long as it is indicated inside the attribute.

[phated]

I was also hoping to discuss writing the PPX in Reason. I think it'd really help contributions. I understand that it's probably desirable to keep an OCaml version around to make it easier to bring into BuckleScript.

Maybe we could set up something like the BuckleScript-TEA project that keeps 2 copies of the project and has scripts that just refmt between them?

[wegry]

Can components annotated with the new ppx be used within components created with older versions of the ppx (< 0.6)?

[bordoley]

We could changing the default component name from make to component. I'll admit a preference for this as components would then be defined as nouns as opposed to verbs in this design. We could also augment the component annotation with a flag that could be used to autogenerate a backwards compatible ReasonReact make function. This would make switching between old and new JSX easier. Particularly as legacy components will likely continue to exist in existing projects.

[rickyvetter]

@wegry: Yes. Either version can be used within the other with some constraints. Only one version of JSX can be used in a file. So if you want to render a record and a function component in the same file, you will need to write out the call manually for at least one (primitive components like div are not included in this - can be used with either).

/* one of these has to be written out manually: */
<>
  <MyRecordComponent/>
  <MyFunctionComponent/>
</>

[phated]

@bordoley That's basically where I've ended up as well. I think using the component name as the default instead of make is the best path forward, so we don't need to write configuration files out to the filesystem (which may be a no-go for BuckleScript anyway).

[bordoley]

Does the PPX work in module interface files (.rei)? For product code it doesn’t matter much, but you could imagine library authors wanting to restrict the modules and functions they export.

[rickyvetter]

@bordoley this ppx supports signatures and .rei files. The last example in the initial post (labelled "interface") shows what the syntax looks like. Are you looking for something more specific?

[sync]

we should be able to provide a default url here what do you think ? use case for example is if you are coming from ssr and want to start rendering from a different route.

[rickyvetter]

Wouldn't you do a redirect to the correct route?

[sync]

you mean ReasonReact.Router.push('mynewroute'); instead ?

[rickyvetter]

No I mean on the server side issuing a 302 or whatever type of redirect it should be. It seems like you client and server should always be in agreement about how URLs map to routes and how that wraps to data.

Maybe I'm not understanding what you mean by "want to start rendering from a different route" - can you expand?

[sync]

want to avoid this:
https://pennyworth.dblechoc.now.sh/more can you see the flashing ?

[sync]

express middleware

let app = <App client={client}  initialPath={here i can get in from the req knowing i am insdie an express middleware} />;
    // first render for ssr cache
    renderToStaticMarkup(app);

my actual component

[@react.component]
let make = (~client, ~inititalPath) => {

 // if i could pass an inititalPath here would be good
  let url = ReasonReactRouter.useUrl();

  <div>
    <GraphqlHooks.Provider value=client>
      <main>
        {switch (url.path |> mapPathToRoute) {
         | Home => <Home />
         | More => <More />
         }}
      </main>
    </GraphqlHooks.Provider>
  </div>;
};

[rickyvetter]

I see what you mean. Yeah I think this is reasonable. I think that serverValue or staticValue might be a better term - initial implies it's fixing an issue with the client not being able to pick up the URL when it first loaded, but it's actually an issue with getting the path via browser APIs on the server. But yes, adding a param here makes sense.

[sync]

awesome thank you

[sync]

see #358

[rickyvetter]

@bordoley thought more about the open React.DOM; thing and I think it would make adoption quite painful. Since lowercase components are treated differently you'd need to remember to open when you switched the bs.config. This is enough added friction that I'm worried about doing it at the same time. I think we can still unify everything down the road.

[rickyvetter]

@phated I think we are going to stick with make - this it's a tough decision as it would make working with both apis easier and would be helpful for the case where you have a canonical renderer for a piece of data, but I think it would hurt initial adoption for new folks as make is already so widely used in this context already and there are relatively straightforward escape hatches with <Foo.component />.