Append each child as argument to React.createElement

f-f / purescript-react-basic-native

Apache License 2.0

60 stars 18 forks source link

Append each child as argument to React.createElement #14

Closed jvliwanag closed 5 years ago

jvliwanag commented 5 years ago

Gets rid of warning that each child should have key
Most of the time hiding this warning is what we want. Problem is in purescript-react-basic we do not differentiate between:

<Foo children={myChildren}/>

and

<Foo><Child1/><Child2/></Foo>

Since both will have the purescript api:

foo_ :: [Array JSX] -> JSX

We mostly use the purescript api to mean the latter JSX case wherein we don't want the warning to appear. So we translate the internal unsafeCreateNativeElement to reflect this.

dwhitney commented 5 years ago

Ugh - I've done a ton of work in the events branch that I was hoping to finish next week and merge, which is why I've never published this on Pursuit. I've a pretty bad head cold today, but I will review these pull requests next week and either merge them or incorportate the work to achieve what you are doing - looks like good stuff to me so far.

dwhitney commented 5 years ago

Hey also I felt that last comment came off a bit negative. I do really appreciate the work you put in to make this better! I definitely want to include the work you did. I was just a little disappointed in myself for not getting the work I had already done into master sooner, so you could have worked against that

joelmccracken commented 5 years ago

TY both for your work!

jvliwanag commented 5 years ago

No worries. I don’t mind rebasing my pull reqs once your branch gets merged. I’ll have a few more over the next week as I’m working through our react native app.

jkachmar commented 5 years ago

Hey @dwhitney what's the status on the stuff in the events branch, and is there anything that I (or anyone else) could do to help get that merged in and this published?

dwhitney commented 5 years ago

@jkachmar hey! I've had a lot of things get in the way at work, but I am now at a point to take a look at this. I'll work on it today and figure out where I was and reply to this thread later today with more info

dwhitney commented 5 years ago

alright the events branch is now compiling with almost no foreign data imports (just CSS) ! Things left to do

StyleProps types instead of CSS
move the components and related props to their own module
move the events to their own module
move the "ART" components to an "ART" module
merge this PR
Try this out. Right now it compiles. Not really sure about anything else
Upgrade to newset version of spago and React Basic

I think I can have this done around Monday.