fable-compiler / fable-react

Fable bindings and helpers for React and React Native
MIT License
275 stars 67 forks source link

Unify type for React components and implement memo for SSR #114

Closed alfonsogarciacaro closed 5 years ago

alfonsogarciacaro commented 5 years ago

In #112 @vbfox has added bindings for React.memo and also some types and helpers to handle references to React components. There was already a helper to instantiate a component from a direct reference, but it's true it wasn't used a lot and the only way to declare a ComponentClass was by typing and imported element from JS. Should we deprecate the from helper in favor of the new ReactElementType module?

And about SSR, what would the equivalente to ReactElementType and how can we make the memo bindings compatible? @zaaack @thinkbeforecoding

zaaack commented 5 years ago

@alfonsogarciacaro For SSR, I think we can simply add a DotNetCore polyfill via a wrapper type, like DU or class, if we can change memo functions' return type to ReactElementType<'props>, I think a DU type is enough; if we really need to return a ReactComponentType<'props>, then we might need a class to extends ReactComponentType. Then we can destructure the real data from ReactElementType.ofXXX and call createServerElementByFn or createServerElement on the server side.

Here is a quick example via DU, not test it yet. I'm kind of busy these days, so anyone who is interesting can take this PR. : )


type ReactElementType<'props> = interface end
type ReactComponentType<'props> =
    inherit ReactElementType<'props>
    abstract displayName: string option with get, set

type ReactElementTypeWrapper<'P> =
| Comp of obj
| Fn of ('P -> ReactElement)
| HtmlTag of string
with interface ReactElementType<'P>

[<RequireQualifiedAccess>]
module ReactElementType =
    let inline ofComponent<'comp, 'props, 'state when 'comp :> Component<'props, 'state>> : ReactElementType<'props> =
        Comp (typeof<'comp>) :> ReactElementType<'props>
     let inline ofFunction<'props> (f: 'props -> ReactElement) : ReactElementType<'props> =
        Fn f :> ReactElementType<'props>
     let inline ofHtmlElement<'props> (name: string): ReactElementType<'props> =
        HtmlTag name :> ReactElementType<'props>
     /// Create a ReactElement to be rendered from an element type, props and children
    let inline create<'props> (comp: ReactElementType<'props>) (props: 'props) (children: ReactElement seq): ReactElement =
#if FABLE_COMPILER
        createElement(comp, props, children)
#else
        match (comp :?> ReactElementTypeWrapper<'props>) with
        | Comp obj ->
            createServerElement(obj, props, children, ServerElementType.Component)
        | Fn f ->
            createServerElementByFn(f, props, children)
        | HtmlTag obj ->
            createServerElement(obj, props, children, ServerElementType.Tag)

#endif
let memo<'props> (name: string) (render: 'props -> ReactElement) : ReactElementType<'props> =
#if FABLE_COMPILER
    render?displayName <- name
    reactMemo(render)
#else
    Fn render :> ReactElementType<'props>
#endif
vbfox commented 5 years ago

On the ComponentClass / ReactElementType i'm not really attached to keeping the custom type and could have reused the existing one from the TS imports we could revert to that too (And rename the module). To be fair I was expecting discussion on that in the PR :)

They are a bit more confusing for users (Because they try hard to map to what react does in JS even if imperfectly) but they represent the same thing.

I didn't notice the from helper, the name seem very generic but whatever direction we go we can implement it in term of the helper class.

alfonsogarciacaro commented 5 years ago

Yes, we should have had a discussion. But I thought of it after merging (and pushing a release). This is what happens when you cannot sleep after your :baby: wakes you up at 3:30AM and then think it's a good idea to use the time to clear GH issues 😬

I think nobody is using from and I agree the name is not good, it should be safe to deprecate it. I don't like very much the name ComponentClass either (particularly now that React is moving away from classes). At one point we'll probably focus on the custom helpers and forget about the bindings translated from TS (Fable.Import.React) which probably are not updated anyway, so I'm OK with keeping the new ReactElementType (we'll have to partition Fable.Helpers.React file soon as it's growing a lot).

Thanks a lot for the code @zaaack. It looks as if should work, I'll use to send a PR :clap: :clap:

alfonsogarciacaro commented 5 years ago

I deprecated from and added @zaaack suggestion for SSR. I hope I did it right!

cmeeren commented 5 years ago

Now that from has been deprecated, how should we interface with libraries that return ComponentClass, such as Material-UI's withStyles? Using deprecated constructs causes some hiccups during compilation. I have replaced from myComponentClassInstance with ReactElementType.create !!myComponentClassInstance, which seems on the surface to work fine, is that a fair workaround?

alfonsogarciacaro commented 5 years ago

@cmeeren The deprecation warning gives you a hint: Use ReactElementType.create, though this requires you to define the imported components as ReactElementType<'props> instead.

let importedComponent: ReactElementType<obj> = import "default" "material-ui/button"

let view props =
    ReactElementType.create props []

Remember you can use ofImport directly:

let view props =
    ofImport "default" "material-ui/button" props []

For material-ui, another alternative is to use Fable Material-UI.

Hope it helps!

cmeeren commented 5 years ago

Thanks. I am indeed using Fable Material-UI, which defines the return value of withStyles as ComponentClass, which is why I asked. :) Until Fable Material-UI changes, is my workaround safe? I.e.:

ReactElementType.create !!myComponentClassInstance
alfonsogarciacaro commented 5 years ago

Yes, as commented above ReactElementType<'props> is basically the same as ComponentClass<'props> so until Fable Material-UI it's OK to use the cast :)