fable-compiler / fable-react

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

mountById ok in js, but shows error in editor #166

Closed rfrerebe closed 5 years ago

rfrerebe commented 5 years ago

When using Fable.React.mountById, https://github.com/fable-compiler/fable-react/blob/master/src/Fable.React.Helpers.fs#L399-L400 the editor (VSCode and VS) shows an error because it's missing in it's nuget package.

I disappeared between beta008 and beta007 https://www.fuget.org/packages/Fable.React/5.0.0-beta-008/lib/netstandard2.0/diff/5.0.0-beta-007/

Probably because of cb85b38

I am unsure about what removing FABLE_COMPILER would do for SSR.

MangelMaxime commented 5 years ago

FABLE_COMPILER is needed because mountById can't work on the Server Side.

You can add the FABLE_COMPILER constant to your *.fsproj so your IDE knows that it should pass that constant to the FCS.

alfonsogarciacaro commented 5 years ago

Yeah, this is an issue with packages that are compatible both with Fable and .NET, if you want to make some methods only available in Fable you get errors in the IDE. Probably we should make the method available in .NET and throw a runtime exception in SSR, because most people don't add the FABLE_COMPILER constant in the .fsproj.

Actually, it may not work even by adding the constant. Because the .dll used by the IDE is already compiled without that method. I will publish a new version with the fix.

rfrerebe commented 5 years ago

Thanks. I confirm adding the constant don’t change anything (method is absent from dll)

It should be possible to add an attribute on the method for SSR side marking it Obsolete, so compile time will raise at least a warning.

rfrerebe commented 5 years ago

I just had an other idea. I don’t know if it’s feasible. The best solution from my point of view would be to publish 2 nugets (Fable.React and Fable.React.SSR ? ) packages from the same code but with different constant enabled at compile time.

alfonsogarciacaro commented 5 years ago

Yeah, that's the approach taken by Thot.Json/Thoth.Json.Net. But this brings other complications (keeping in sync the dependencies when sharing code between client and server, etc) and potential sources of confusion for users, so I'd prefer not to do it for now.