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

Easy way to simplify pages, and satisfy FastRefresh #47

Open
mathieutu opened this issue Jun 1, 2021 · 7 comments
Open

Easy way to simplify pages, and satisfy FastRefresh #47

mathieutu opened this issue Jun 1, 2021 · 7 comments

Comments

@mathieutu
Copy link

mathieutu commented Jun 1, 2021

Hi @ryyppy,
Rescript beginner here, I'm digging around nextjs with rescript.
I've seen in all your pages that you can't re-export directly the default, because of fast refresh and the name $$default of the components.

However, if we use the @react.component decorator, the name will not be only default but MyModule$default which totaly satitisfy FastRefresh:

module Label = {
  @react.component
  let make = (~title: string) => {
    <div className="myLabel"> {React.string(title ++ "foo")} </div>
  }
}

@react.component
let default = () => {
  <div>
    <Label title="Getting Started" />
    {React.string("foo")}
  </div>
}

⇣⇣⇣

// Generated by ReScript, PLEASE EDIT WITH CARE

import * as React from "react";

function MyTest$Label(Props) {
  var title = Props.title;
  return React.createElement("div", {
              className: "myLabel"
            }, title + "foo");
}

var Label = {
  make: MyTest$Label
};

function MyTest$default(Props) {
  return React.createElement("div", undefined, React.createElement(MyTest$Label, {
                  title: "Getting Started"
                }), "foo");
}

var $$default = MyTest$default;

export {
  Label ,
  $$default ,
  $$default as default,
  
}
/* react Not a pure module */

My next page is here only:

export { default } from 'modules/MyTest.bs.js'

So why not use the decorator on the default component?

Thanks.
Mathieu.

@ryyppy
Copy link
Owner

ryyppy commented Jun 1, 2021

There are many scenarios where you can't use the @react.component decorator, so it's a pretty prominent footgun. As long as you keep an eye on the JS output, you can of course just export and use the components directly. As soon as you start putting more logic into your components, you may break your fast refresh conditions (e.g. you define a variable in your toplevel .res that will be automatically exported -> this will break the contract.. so you'd need an .resi file to mitigate that).

My motivation with this template is to provide one solution that will reliably work 100% of the time, even when the code changes / grows.

@mathieutu
Copy link
Author

mathieutu commented Jun 1, 2021

Hi, thanks for your answer.

There are many scenarios where you can't use the @react.component decorator

Could you give me some link/doc where I can find some (noob here)?

you define a variable in your toplevel .res that will be automatically exported -> this will break the contract

I've tried but couldn't reproduce the error. Do you have any example where fast refresh is broken?
This works well:

let useCustom = () => {
  let (state, setState) = React.useState(() => "foohhh")
  state ++ "foo"
}

let bar = "bar"


@react.component
let default = () => {
  let msg = useCustom()

  <div>
    <Label title="Getting Started" />
    {React.string(msg ++ bar)}
  </div>
}```


I'm not talking about directly writing the Nextjs page file in res, but just re-exporting the default from res file instead.

eg replace:

```js
import ExamplesRes from "src/Examples.mjs";

// This can be re-exported as is (no Fast-Refresh issues)
export { getServerSideProps } from "src/Examples.mjs";

// Note:
// We need to wrap the make call with
// a Fast-Refresh conform function name,
// (in this case, uppercased first letter)
//
// If you don't do this, your Fast-Refresh will
// not work!
export default function Examples(props) {
  return <ExamplesRes {...props}/>;
}

with

export { getServerSideProps, default } from "src/Examples.mjs";

by adding @react.component on top of Examples.res let default.

Thanks!

@ryyppy
Copy link
Owner

ryyppy commented Jun 1, 2021

Could you give me some link/doc where I can find some (noob here)?

According to the Next docs https://nextjs.org/docs/basic-features/fast-refresh#how-it-works, a module is required to only export React components.

I've tried but couldn't reproduce the error. Do you have any example where fast refresh is broken?

Try this (pretty much on every SSRed page):

type props;
let default = (props: props) => {

  <div/>
}

This will export as $$default, which is not a valid React component name, and therefore wrongly detected as a non-react component. Unless they fixed some bugs in between, I am sure this will deopt fast-refresh. That's why I recommend wrapping the explicitly imported default or make function in a new React component, like described in the template.

@mathieutu
Copy link
Author

mathieutu commented Jun 1, 2021

Sorry, I feel like I can't make myself understood 😅.

  1. I was asking examples where "you can't use the @react.component decorator"

  2. Yeah, if you just write let default, it will not work, but if you decorate it with @react.component it will, as react-rescript will output a function named MyModule$default instead of $$default, which satisfy FastRefresh.

I actually can make you a PR if you want to test it directly.

@ryyppy
Copy link
Owner

ryyppy commented Jun 1, 2021

I was asking examples where "you can't use the @react.component decorator"

E.g. in App.res: https://github.com/ryyppy/rescript-nextjs-template/blob/master/src/App.res#L21

or in any scenario where you need static SSG, or SSR, because you probably want to sync the prop type with your component and your getServerSideProps. You can't use @react.component if you want to provide the prop type yourself.

Like here:
Examples.res: https://github.com/ryyppy/rescript-nextjs-template/blob/master/src/Examples.res#L6

@mathieutu
Copy link
Author

mathieutu commented Jun 1, 2021

you can't use @react.component if you want to provide the prop type yourself.

Ok! This was THE thing I missed. For me, even in this case, you would use labeled arguments for props typing.

So maybe that the effort could be made on rescript-react to allow passing props type to @react.component, instead of fast-refresh?
(talking about various PR and exchanges on forum about that)

This can be something useful, event outside next and fast-refresh.

@ryyppy
Copy link
Owner

ryyppy commented Jun 1, 2021

Yeah there were some ideas on how to make props passing possible in a newer JSX version (v4), but right now we are currently waiting for another specific compiler change before going back into this topic.

Btw, I found some old thread where we discussed some fast-refresh issues as well: https://forum.rescript-lang.org/t/rescript-nextjs-template-2021-update/829

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