search

fable-compiler / fable-react

Fable bindings and helpers for React and React Native
MIT License
273 stars 67 forks source link

Fix #194 #195

Closed alfonsogarciacaro closed 1 year ago

alfonsogarciacaro commented 4 years ago

Ok, this is my new attempt to fix the FunctionComponent.Of API #188 #194. To make sure users are always passing the same render function to each FunctionComponent.Of I'm storing a stringified version of the function in the cache and compare it with newly passed render arguments. Seems to work but please someone give it a try to double check. I'm doing this only in Debug mode to avoid the overhead in production. I hope it's enough but let me know what you think.

This PR also updates npm dependencies to remove the security warnings (there's still one about yargs-parser when running npm audit that I couldn't solve).

MangelMaxime commented 4 years ago

Looking at the code, I think it should work.

I guess we should try to release a beta version this time and ask people having the issue with the upgrade to comment if that helps them or not. Either by fixing the problem or helping troubleshoot the problem :)

GBirkel commented 4 years ago

I think this fixes a rendering issue we’re having with our recent merge at Demetrix...

alfonsogarciacaro commented 4 years ago

Hi @GBirkel! Please note that unfortunately this is not a "fix", just a way to fail fast and warn the user. If you're having problems maybe it's safer to go back to Fable.React 5.x.

@MangelMaxime Should we unlist the 6.x package if users seem to be having issues?

MangelMaxime commented 4 years ago

@alfonsogarciacaro I don't know in general I don't like the unlisting package but if that avoid problem perhaps we should.

Also, now it has been out for several days/weeks so perhaps it is too late?

Releasing this new fast failing would help people make the transition should be enough in theory no?

forki commented 4 years ago

With standard nuget unlisted packages are still installed if they are the latest. It's a bug that they won't fix. So you always need to Unlist + push a new one

Maxime Mangel notifications@github.com schrieb am Sa., 16. Mai 2020, 18:47:

@alfonsogarciacaro https://github.com/alfonsogarciacaro I don't know in general I don't like the unlisting package but if that avoid problem perhaps we should.

Also, now it has been out for several days/weeks so perhaps it is too late?

Release this new fast failing would help people make the transition should be enough in theory no?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/fable-compiler/fable-react/pull/195#issuecomment-629674039, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAOANAWGBMJASI2RL3PJ5LRR27SBANCNFSM4M352OSA .

MangelMaxime commented 4 years ago

@alfonsogarciacaro Should we release this new version?