[WIP] Update to Fable 2

[forki]

cc @alfonsogarciacaro @MangelMaxime

[MangelMaxime]

@forki What are you expecting from me?

For me, the diff seems good for an upgrade to Fable 2.1. Do you expect me to build/test the app?

[forki]

no I don't. I'm currently missing the command that will run fable. https://github.com/SAFE-Stack/SAFE-Nightwatch/issues/64#issuecomment-465573157 basically the replacement for dotnet fable npm-run build

[MangelMaxime]

You need to start fable-splitter by using yarn run fable-splitter for example.

It will start the compiler for you.

[forki]

ah cool. that was missing link. trying that now!

[forki]

So everything compiles, but

https://github.com/react-navigation/react-navigation/issues/2767#issuecomment-347302691 suggest there is some trouble with the import.

https://stackoverflow.com/a/47871724/145701 suggests there needs to be some additonal export!?

But I don't see what changed.

@alfonsogarciacaro any ideas?

[forki]

@MangelMaxime I assume we need to "export" Elmish.ReactNative.Components.App somehow!?

https://github.com/elmish/react/blob/master/src/react-native.fs#L15

[forki]

https://github.com/elmish/react/commit/c6c15b5311dc4e6d75fa9fbf1b5654def0003f8c#diff-ce3e72f76b021b9de55e8aba30e99cf5R45 looks like @alfonsogarciacaro already fixed something for fable2. how can I test that?

[forki]

yes that's the correct change. good work @alfonsogarciacaro just the issue is: there is no compatible package of this released

[MangelMaxime]

@forki If I didn't mess up the merge/rebase, I released Fable.Elmish.React 2.2.0 with the fix included.

Available when nuget ready.

[MangelMaxime]

@MangelMaxime I assume we need to "export" Elmish.ReactNative.Components.App somehow!?

https://github.com/elmish/react/blob/master/src/react-native.fs#L15

I don't understand this comment. Is the fix in Fable.Elmish.React enough or do we still need to do something?

[forki]

fix would be enough. let me test the new packge

[forki]

confirmed. Fable.Elmish.React 2.2.0 fixed it. thanks!

now I'm down to warnings.

it complains about line 59 in

[forki]

 const logo = createElement(Image, createObj(ofArray([new Props.ImageProperties(5, require("../../images/raven.jpg")), new Props.ImageProperties(6, ofArray([new Props.FlexStyle(1, "center"), new Props.FlexStyle(10, "column")]))]), 1));

this is how fable did it in 1.x

[alfonsogarciacaro]

Looks like the keyValueProps call is missing. Fable 1 did this implicitly I believe, but in Fable 2 you have to call it explicitly to transform the list into a JS object

[alfonsogarciacaro]

Ah, I think the Style function is wrong, I'll check after coming back from kindergarten

[forki]

so we need to update the bindings?

[alfonsogarciacaro]

I think so, in fable-react, Style is disguised as a union case, but it's actually a function that calls keyValueList: https://github.com/fable-compiler/fable-react/blob/b89835014ae16caec51938ad8337010b36b13613/src/Fable.React/Fable.React.Props.fs#L731-L732

We need to do something similar for fable-react-native bindings, for example here: https://github.com/fable-compiler/fable-react-native/blob/e4c666f27dfc1ff77bdbfaa0e899f30dadff2839/src/Fable.Helpers.ReactNative.fs#L531

Because there are many cases accepting IStyle list, if this makes the bindings too hacky, another solution is to create a function like this:

type IStyleProps = interface end

let style (props: IStyle seq) = keyValueList CaseRules.LowerFirst props :?> IStyleProps

Then change all IStyle list to IStyleProps. The drawback is it forces the user to call style all the time, but you can turn the function into an operator like: let (!^) (props: IStyle seq) = ...

[forki]

Tbh I don't mind making it breaking (it's already breaking anyway) - I want it look work as close as possible like React proper.

Alfonso Garcia-Caro [email protected] schrieb am Fr., 22. Feb. 2019, 10:44:

I think so, in fable-react, Style is disguised as a union case, but it's actually a function that calls keyValueList: https://github.com/fable-compiler/fable-react/blob/b89835014ae16caec51938ad8337010b36b13613/src/Fable.React/Fable.React.Props.fs#L731-L732

We need to do something similar for fable-react-native bindings, for example here: https://github.com/fable-compiler/fable-react-native/blob/e4c666f27dfc1ff77bdbfaa0e899f30dadff2839/src/Fable.Helpers.ReactNative.fs#L531

Because there are many cases accepting IStyle list, if this makes the bindings too hacky, another solution is to create a function like this:

type IStyleProps = interface end let style (props: IStyle seq) = keyValueList CaseRules.LowerFirst props :?> IStyleProps

Then change all IStyle list to IStyleProps. The drawback is it forces the user to call style all the time, but you can turn the function into an operator like: let (!^) (props: IStyle seq) = ...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/SAFE-Stack/SAFE-Nightwatch/pull/66#issuecomment-466337499, or mute the thread https://github.com/notifications/unsubscribe-auth/AADgNISqEQ9flokMjFB1KxZPfu-HrBLIks5vP7vjgaJpZM4bFKNN .

[forki]

let's go with option 1: what do I need to do?

[forki]

@alfonsogarciacaro

for testing I added the following to my Fable.Helpers.ReactNative.fs:

let inline ImagePropertiesStyle (style: IStyle list) : IImageProperties =
    !!("style", keyValueList CaseRules.LowerFirst style)

and indeed it seems to work if I use that function instead of ImageProperties.Style.

so we replace all these union cases with such a function? Is that the idea?

[MangelMaxime]

@forki Yes, the idea is to replace the *.Style to use a similar function.

If the existing DSL used a Style case, you can use a static member and make it a non breaking change. Example

[forki]

Ok cool. Will try it and send pull request

Maxime Mangel [email protected] schrieb am Fr., 22. Feb. 2019, 14:01:

@forki https://github.com/forki Yes, the idea is to replace the *.Style to use a similar function.

If the existing DSL used a Style case, you can use a static member and make it a non breaking change. Example https://github.com/fable-compiler/fable-react/blob/62c0c88830c72e11bab32ad9686fb8aa9ef1289a/src/Fable.ReactLeaflet/ReactLeaflet.fs#L472

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/SAFE-Stack/SAFE-Nightwatch/pull/66#issuecomment-466389057, or mute the thread https://github.com/notifications/unsubscribe-auth/AADgNJR9PzuzSoxzlYpl5L9X8j5Jbgi4ks5vP-pDgaJpZM4bFKNN .

[forki]

@alfonsogarciacaro @MangelMaxime looks like we actually made it work. Thanks guys!

Only issue left is bug in fable splitter: https://github.com/fable-compiler/Fable/issues/1751

[ImaginaryDevelopment]

Is this work still ongoing? Can we use DotNet 5 and the latest Fable for react-native apps?

[tforkmann]

I have a open PR for net5 and Fable 3 Works on iOS Android need a tiny fix