fable-compiler / fable-react

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

Feature/fix void elements #24

Closed MangelMaxime closed 7 years ago

MangelMaxime commented 7 years ago

I tried several definition for voidEl function.

let inline voidEl (tag: string) (props: IHTMLProp list) : ReactElement = 
    applySpread createEl (tag, keyValueList CaseRules.LowerFirst props) // No props applied
    applySpread createEl (tag, keyValueList CaseRules.LowerFirst props, null) // Runtime error due to null
    applySpread createEl (tag, keyValueList CaseRules.LowerFirst props, []) // This is working

Fix #24

MangelMaxime commented 7 years ago

And I confirm that this change generate errors by the compiler.

alfonsogarciacaro commented 7 years ago

Maybe something like let inline area b = domEl "area" b [] would work but this is also fine :+1: I wonder why applySpread createEl (tag, keyValueList CaseRules.LowerFirst props) didn't apply the props, I guess I need to fix applySpread (this is just a hacky function I wrote ad hoc for the React bindings).

Thanks a lot!

MangelMaxime commented 7 years ago

Yes, would be nice to see if the problem come from applySpread as often less is better. :)

alfonsogarciacaro commented 7 years ago

I had a look, this is how applySpread works, basically it checks the last argument: if it's a list constructor, it applies the list elements as individual arguments (applySpread fn(foo, [1; 2; 3]) becomes fn(foo, 1, 2, 3)), while for other cases it applies the ES2015 spread operator (applySpread fn(foo, bar) becomes fn(foo, ...bar)).

That's why props fails if it becomes the last argument. Unfortunately this is necessary because users can pass list references so we cannot destruct the individual elements at compile time.

MangelMaxime commented 7 years ago

Ok :) If there is a requirement then we can let it like that :)

@alfonsogarciacaro One question did you publish a new version on npm ?

alfonsogarciacaro commented 7 years ago

@MangelMaxime I just did! npm i fable-react@next