ohanhi / elm-native-ui

[CLOSED] Experiment: mobile apps in Elm using React Native.
BSD 3-Clause "New" or "Revised" License
1.54k stars 75 forks source link

WIP: Use a proper main #24

Closed eeue56 closed 8 years ago

eeue56 commented 8 years ago

Based off of @ohanhi's discussion with Evan, we can use a fork of elm-compiler with this project. This fork can be found here. Binaries will be coming soon for those who ask.

The premise is that all bootstrapping code should be moved into a Elm Native module.

It's likely that Core will also needed to be forked. I can do that too, if someone breaks down for me:

ohanhi commented 8 years ago

@eeue56 My goal is to make the whole thing analogous with how elm-html works. In my mind, StartApp should work unmodified with this. Things currently get drawn in the index.ios.js, basically here. Does this answer your questions?

yusefnapora commented 8 years ago

I think that a slight modification to StartApp will be needed, since we'd want to return a Signal VTree instead of a Signal Html as StartApp currently does. But I think that's probably all that needs to change.

eeue56 commented 8 years ago

making startapp work is trivial, just need to use the NRI fork. The bit that is complicated the bootstrapping bit in the runtime file

yusefnapora commented 8 years ago

@eeue56 I managed to get a working implementation last night. It's at my vtree-main branch and depends on this fork of elm-core.

It uses a very similar technique as the Elm virtual-dom package, in that the Node type is backed directly by React elements, rather than being an ADT which gets converted into React elements later.

I think it might be possible to get the ADT type to work as well, but it would require a lot of awkward javascript to inspect the vtree's ctor and extract the values, etc. since main is returning the VTree type directly without encoding to json. I might be overlooking a simpler approach though :)

The elm-core fork adds an Elm.embedReact() function that works similarly to Elm.embed(). It has three required args:

And you can pass in a fourth argument, which is the optional object of inputs as with Elm.fullscreen and Elm.embed

Probably needs some cleanup before it's ready to go, but hopefully this'll be a decent start :)

eeue56 commented 8 years ago

:+1: looks good

eeue56 commented 8 years ago

an object containing a React field, whose value is the output of require('react-native')

I don't think this is a good idea. If you want to require things like this, just use import Native.ReactNativeJS instead, where ReactNativeJS is the JS file to be imported

eeue56 commented 8 years ago

It uses a very similar technique as the Elm virtual-dom package, in that the Node type is backed directly by React elements, rather than being an ADT which gets converted into React elements later.

One of the biggest problems with this approach is that you can no longer use things like toString or show on the vdom elements, leading to runtime errors. Since there currently is no elm-native-ui legacy projects, you should instead try a different approach IMO. This is a lesson I learnt from doing server-side Elm, where I needed to debug things a lot. Having toString work or other things we expect from Elm is very handy :)

eeue56 commented 8 years ago

Also, grab https://github.com/NoRedInk/elm-compiler/tree/elm-native-ui and see if it works and let me know if it doesn't

eeue56 commented 8 years ago

Also - the diff is very noisy! Can you make it without the elm-format changes?

yusefnapora commented 8 years ago

Sweet, thanks for checking it out :)

I think you're right that backing with react elements directly is buying trouble down the road... I went that route because I was using virtual-dom as a reference, but I'd rather keep things in "elm land" as much as possible.

I'll take another pass through and try to get the ADT approach working, and hopefully get rid of the need to pass in the reactEnvironment object with the require'd React components.

Sorry about the noise, I'll switch off elm-format and try splitting the diffs a bit for the next round :)

yusefnapora commented 8 years ago

Btw, the forked compiler worked great; I just had to update the elm-package.json's repository field to https://github.com/elm-native-ui/elm-native-ui.git so the compiler would be happy.

yusefnapora commented 8 years ago

I made a new branch that keeps the VTree type as an elm ADT until it hits the Native.ReactNative.render function, which is only accessed by elm-core's Runtime.js. So inside of Elm, you should be able to call toString, etc. on a VTree.

I also removed the reactEnvironment arg to Elm.embedReact in the elm-core fork. This means that there's not yet a way to render custom JS components, but I'm fine with leaving that out until a clean solution is found.

There's still one opaque, native-backed type: NativeValue is used for event handler functions, and there's a NativeProperty data constructor that accepts a NativeValue. I can't think of another way to attach event handlers directly to the VTree type... But if there's a cleaner way, let's go for it.

ohanhi commented 8 years ago

The newest iteration is looking awesome, @yusefnapora! Great work! Could we maybe get this stuff merged within a couple of days, so the live coding session could already be based on the new solution? :)

yusefnapora commented 8 years ago

That sounds good to me @ohanhi :) I'll hopefully have some time tonight to go over the comments you and @eeue56 made and do a bit of cleanup.

eeue56 commented 8 years ago

@yusefnapora is there anything I need to do to help or have you got it?

yusefnapora commented 8 years ago

@eeue56 if you want to review the last few commits, that'd be sweet. I put up #25 to collect the diffs and make it a bit easier to track changes. I think it's in pretty decent shape, but it would be nice to update the README with a note about the compiler and elm-core forks.

Thanks for your earlier comments & reviews, btw; they were very helpful!