Skip to content
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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate preload code from the component module #48

Closed
cometkim opened this issue Apr 28, 2020 · 9 comments
Closed

Separate preload code from the component module #48

cometkim opened this issue Apr 28, 2020 · 9 comments

Comments

@cometkim
Copy link
Contributor

cometkim commented Apr 28, 2020

Hi @zth,

I'm trying to clone relay's issue tracker example into reason-relay here (to learn ReasonML, and now I'm thinking ReasonRelay is best of Relay at this moment)

Currently, I'm having difficulty implementing Concurrent UI Pattern (a.k.a Render-as-you-fetch) in reason-relay. The problem is preload function is colocated with the component in reason-relay. When implementing Concurrent UI pattern, I wanna execute component lazy loading and data preloading in parallel just like this, but this isn't possible because calling preload requires importing component module synchronously.

So It seems should be moved or duplicated the function into the generated artifacts.

let loadRoute = (route: t) =>
  switch (route) {
  | Home => {
      component:
        ReactCache.make(
          () => {
            DynamicImport.(
              import("./HomeRoot.bs.js")->resolve
              <$> (
                (module Component: PreloadableComponentType) => Component.make
              )
            )
          },
          None,
        ),
      data:
        Home(
-          HomeRoot.HomeRootQuery.preload(
+          // This should be:
+          HomeRootQuery_graphql.preload(
+          // So BuckleScript import only query code, not the component code above.
            ~environment=RelayEnv.environment,
            ~variables={owner: "facebook", name: "relay"},
            ~fetchPolicy=StoreOrNetwork,
            (),
          ),
        ),
    }
@zth
Copy link
Owner

zth commented Apr 28, 2020

Hi @cometkim !

Really really awesome that you're experimenting with this! 😃 I actually spent some time on this a few weeks back, and while I never got around to publishing my work (nor actual code splitting which you're doing and would be really cool to incorporate into this), I just pushed the work I had done previously to a new branch: https://github.com/zth/reason-relay/compare/router?expand=1

What I've done so far is really unpolished and there's at least a few things I'm going to change, but maybe it can serve for some inspiration. It's also a port of the issue-tracker router, and the idea is to ship a basic and flexible router with reason-relay that's concurrent mode/suspense/ssr/preload ready, in an easy to use and type safe way.

Aaaanyway, back to your original question. Yes, for sure preloading should be moved into the actual generated module so it's not tightly coupled with the definition, just like you describe. I've done some prototyping on it, let me see if I can get that done and released quickly so you can continue your experimentation, which is really interesting and cool 👍

@zth
Copy link
Owner

zth commented Apr 28, 2020

@cometkim are you using VSCode by any chance? If so, I'd love for you to test drive a dedicated VSCode extension for ReasonRelay I'm working on if you're up for it.

It's a bit rough around the edges still since it's in testing, but it should hopefully improve your experience. It's called vscode-reason-relay and is published (although I haven't announced it yet) and available in the VSCode Extensions Marketplace. If you're interested in trying it out, I recommend reading through the Readme before you get started to learn about the features. If you have my other VSCode extension for ReasonML + GraphQL installed (vscode-reasonml-graphql) then you can remove that one as this supercedes that.

@cometkim
Copy link
Contributor Author

cometkim commented Apr 28, 2020

@zth Yeah! I used your VSCode extension as well and I was surprised that the relay compiler was very seamlessly integrated. (And wondering that other relay compilers still don't offer better DX like this)

I'm satisfied with ReasonML's module system, PPX, and strong type inference are a perfect fit for codegen-based processes. Even way faster, TypeScript is terribly slow when used with codegen (AND it now has better IDE-support? 🤯)

I'm doing it slowly because I'm a newbie at ReasonML. Obviously, it would be better if you had a well-integrated router something like navi as a first-class citizen (or third-party plugin would be fine). I still don't know how to properly represent route variants, and how to support nested routes. 🤷

I will let you know on Twitter if there is any progress or any question. 😉

@zth
Copy link
Owner

zth commented Apr 28, 2020

@cometkim I just published 0.8.3 which now supports what you want. Check this out: https://github.com/zth/reason-relay/blob/e3eca6151021c3c59ededca71b68a7b2ec4043bb/packages/reason-relay/__tests__/Test_query.re#L125-L128

I can hopefully get back to you soon-ish about the routing. Routing is always a hassle to get right, but I have some ideas for what I want to support in an initial release.

Great to hear that you're trying ReasonML out, and yes, please do let me know of any issues you bump into! Also please post issues on the repo of the extension if you find things that annoy you, don't work etc.

Really looking forward to follow your progress on this experimentation! 🎉

@zth
Copy link
Owner

zth commented Apr 28, 2020

Fixed in 6e29f8a

@zth zth closed this as completed Apr 28, 2020
@cometkim
Copy link
Contributor Author

Thank you for the quick response!

@cometkim
Copy link
Contributor Author

@zth Oops, vscode-reason-relay overwrites new artifacts with older versions. Is there a way to make it use the workspace version?

@cometkim
Copy link
Contributor Author

Ah, never mind. It seems resolved after reloading vscode. 😅

@zth
Copy link
Owner

zth commented Apr 28, 2020

@cometkim please open an issue for that if you don't mind. I think it's just about restarting the compiler but that could be really confusing.

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

No branches or pull requests

2 participants