reasonml / reason-react

Reason bindings for ReactJS
https://reasonml.github.io/reason-react/
MIT License
3.25k stars 348 forks source link

PPX proposal: React.lazy #498

Open bloodyowl opened 5 years ago

bloodyowl commented 5 years ago

it's currently pretty hard to get a type-safe dynamic import using BuckleScript. it's even harder to use React.lazy.

how would you feel about having a PPX:

module ReactLazy = {
  [@bs.module "react"]
  external make:
    (unit => Js.Promise.t({. "default": React.component('a)})) =>
    React.component('a) =
    "lazy";
};

[@react.lazy]
module MyComponentLazy = MyComponent;

// that would expand to something like:
module MyComponentLazy: MyComponentType = {
  module type MyComponentType = (module type of MyComponent);
  [@bs.val] external myComponent: (module MyComponentType) = "undefined";
  include (val myComponent);
  [@bs.val]
  external import:
    ([@bs.as "MODULE_RELATIVE_PATH"] _, unit) => Js.Promise.t(module MyComponentType) =
    "import";
  let make =
    ReactLazy.make(() =>
      import()
      |> Js.Promise.then_((module MyComponent: MyComponentType) =>
           Js.Promise.resolve({"default": MyComponent.make})
         )
    );
};
ozanmakes commented 5 years ago

I haven't had the chance to use React.lazy or React.Suspense yet, and haven't tested the following code, but I gave it a shot at approximating your example transformation with reason-macros:

// MyComponent.re
[@react.component]
let make = (~title=?, ~children) => <div ?title> children </div>;
// MyComponentLazy.re
%react.lazy
(MyComponent, "./MyComponent.bs.js");
// Shared.macros.re
[@macro.name "react.lazy"]
let%macro.toplevel _ =
  (moduleName: capIdent, modulePath: string) => [%str
    include (
      {
        [@bs.val]
        external import:
          ([@bs.as "$eval{modulePath}"] _, unit) => Js.Promise.t('a) =
          "import";

        [@bs.module "react"]
        external lazy_: (unit => Js.Promise.t('a)) => 'a = "lazy";

        let makeProps = Eval__moduleName.makeProps;
        let make =
          lazy_(() =>
            import()
            |> Js.Promise.then_(x =>
                 Js.Promise.resolve({"default": x##make}) |> Obj.magic
               )
          );
      }:
        (module type of {
        let makeProps = Eval__moduleName.makeProps;
        let make = Eval__moduleName.make;
      })
            )
  ];
// Generated MyComponentLazy module type

{
  let makeProps:
    (~title: 'a=?, ~children: 'b, ~key: string=?, unit) =>
    {. "children": 'b, "title": option('a)};
  let make:
    {. "children": React.element, "title": option(string)} => React.element;
}
// Generated MyComponentLazy.bs.js

// Generated by BUCKLESCRIPT, PLEASE EDIT WITH CARE

import * as React from "react";
import * as Caml_option from "../node_modules/bs-platform/lib/es6/caml_option.js";

function makeProps(prim, prim$1, prim$2, prim$3) {
  var tmp = {
    children: prim$1
  };
  if (prim !== undefined) {
    tmp.title = Caml_option.valFromOption(prim);
  }
  if (prim$2 !== undefined) {
    tmp.key = Caml_option.valFromOption(prim$2);
  }
  return tmp;
}

var make = React.lazy((function (param) {
        return import("./MyComponent.bs.js").then((function (x) {
                      return Promise.resolve({
                                  default: x.make
                                });
                    }));
      }));

export {
  makeProps ,
  make ,

}
/* make Not a pure module */

Would've been easier to do (and also allow us to keep makeProps as external) if reason-macros transformed module names on the right hand side. For example, module type Foo = module type of Eval__name or include Eval__name. But I think something like this could do the trick until this issue is resolved.

rickyvetter commented 4 years ago

I like this concept but we can't implement it until we get a reliable and typesafe way to define MODULE_RELATIVE_PATH. If we have a way to do this then it's worth building!

bloodyowl commented 4 years ago

@bobzhang would that be hard to implement in BuckleScript?

rickyvetter commented 4 years ago

BTW done a bit more work to research this and I want to document the three things I believe we need from BuckleScript:

  1. A function that goes from MyModule to Js.Import.t((module typeof MyModule)). Suggested sytax: [%import MyModule]. This function should compile-time error if you pass a module that isn't a static, file-level construct which has 2:
  2. We need an annotation for files to opt in to dynamic imports. This might look like [@allowDynamicImports] at the top module level. This is because a number of compiler optimizations need to be disabled in these modules. [@bs.module] external foo: int = "myModule"' needs to be converted to a concrete value in the module. let foo = MyOtherModule.foo;' needs to never be short circuited at callsites to avoid statically importing these files instead of using through the dynamic import process.
  3. A function external import: t('module) => Js.Promise.t('module)

Two might seem odd given the way dynamic imports in JS work today, but I think it's actually an improvement as a module author you have more control over how this module is used. This might be able to be done without the annotation, but I think it would be much harder and I am not sure the tradeoffs are worth it.

zth commented 4 years ago

@rickyvetter thanks for sharing your research, this is really exciting!