liqula / react-hs

A GHCJS binding to React based on the Flux design. The flux design pushes state and complicated logic out of the view, allowing the rendering functions and event handlers to be pure Haskell functions.
32 stars 10 forks source link

Make react-hs compile with GHC #17

Closed tysonzero closed 7 years ago

tysonzero commented 7 years ago

So currently I can't use ghci or hdevtools with react-hs.

I can't use ghci since ghcjs doesn't support it, or at least the version of ghcjs version specified here doesn't.

I can't use hdevtools since hdevtools doesn't support ghcjs.

I am trying to figure out how to get a ghcjs version with ghci that works with stack, and I also added an issue to hdevtools for ghcjs support. But I do think that react-hs should support ghc.

A library that might help a lot is ghcjs-base-stub.

tysonzero commented 7 years ago

18

fisx commented 7 years ago

Thanks! Good to know this exists, but how does it make ghc compile the foreign function syntax?

Have you tried it and confirmed it solves your problem? If so I'll test-run and then probably merge it.

(Btw I just realized there is another react binding. Do you know anything about https://www.stackage.org/package/glazier-react?)

tysonzero commented 7 years ago

So with my change it looks like there have been no degradations in the sense that it still compiles with GHCJS just fine, however it still does not compile with GHC. It seems like it is a step in the right direction, but it still gives me the following error message, and I have not interacted enough with JS FFI / FFI in general to know how to fix it:

 Configuring react-hs-0.0.1...
    Preprocessing library react-hs-0.0.1...
    [ 1 of 12] Compiling React.Flux.Internal ( src/React/Flux/Internal.hs, .stack-work/dist/x86_64-osx/Cabal-1.24.2.0/build/React/Flux/Internal.o )

    .../.stack-work/downloaded/ufTvVgsUDzR4/react-hs/src/React/Flux/Internal.hs:612:1: error:
        • The `javascript' calling convention is unsupported on this platform
        • When checking declaration:
            foreign import javascript unsafe "static $2[$1]" js_findFromArray
              :: Int -> JSVal -> JSVal

    .../.stack-work/downloaded/ufTvVgsUDzR4/react-hs/src/React/Flux/Internal.hs:612:1: error:
        • ‘$2[$1]’ is not a valid C identifier
        • When checking declaration:
            foreign import javascript unsafe "static $2[$1]" js_findFromArray
              :: Int -> JSVal -> JSVal

Any idea how to fix this issue?

I have unfortunately not looked into glazier-react at all. Could potentially look into it later.

Also do you have a contact email? I don't see one attached to your GitHub profile.

fisx commented 7 years ago

That's what I suspected: The ghcjs-base-stub gives you all the functions in ghcjs-base in a form that ghc can digest, but it doesn't magically make ghc support foreign functions.

So at least for those I guess you would have to use the CPP trick that was used in react-flux.https://bitbucket.org/fisx/react-flux/commits/c4f7af6c6c708dfc14cb05b19b15e470d0856826?at=default.

I would like to see all foreign functions moving to the bottom of their modules if this is re-introduced. Then at least we only need one CPP switch per module.

tysonzero commented 7 years ago

Alright sounds good, I will work on implementing the CPP trick.

tysonzero commented 7 years ago

Ok I have implemented the CPP trick, with all the #ifdef stuff at the bottom of the file. This change makes it compile just fine with GHC, although you will have to specify which version of ghcjs-base-stub you want in stack.yaml when compiling with GHC.

fisx commented 7 years ago

LGTM.

Could you rebase to or merge (whichever you prefer) the current master? Let me know if you would like me to help, but I think it's just this commit that's in the way: https://github.com/liqula/react-hs/pull/19/commits/25af199c7df9ce01e12d1fba6c7826d32ebb43dc

And do you want to add a Section "building with GHC" to the README and do react-hs-servant, react-hs-examples, too? (It's ok if not, this is an improvement either way.)

tysonzero commented 7 years ago

I merged the current master, should be good to go now! I'd say go ahead and merge this branch as is, but sometime soon I want to do a thorough look through the README and examples and hopefully make quite a few improvements. Including mentioning compiling with GHC.

fisx commented 7 years ago

That would be great!

(Related: https://github.com/ghcjs/ghcjs/pull/522.)