search

thoth-org / Thoth.Elmish.Toast

https://thoth-org.gitbook.io/thoth-elmish-toast/
MIT License
6 stars 4 forks source link

[WIP] Update CustomEventInit to handle generic type #31

Closed PaigeM89 closed 3 years ago

PaigeM89 commented 3 years ago

This is a first pass at resolving #30. This passes along more generic information to prevent them from being constrained to object, and specifies the generic type for CustomEventInit.

However, there is currently a bug where the generated JS is something like this:

new (CustomEvent())("thoth_elmish_toast_notify_event", detail$$3);

Which is invalid because CustomEvent() doesn't have a new keyword.

I'm not sure if this is because of my changes here, the code I tested with that's running this (I can't figure out how to get Demo to start), or - my actual guess - something upstream. I'm opening the MR to get some eyes on this, hopefully I've missed something simple that'll resolve this faster.

MangelMaxime commented 3 years ago

Hello @PaigeM89,

the problem is because the generated code is invalid.

Indeed, new is required when created a custom event:

image

The problem is that Fable generate an valid code here because of how it scope the code using ().

I will take a look at the Browser.Event binding to see if there is a way to fix it their or if this problem need a fix in Fable directly.

MangelMaxime commented 3 years ago

The problem has been reported to Fable repository and it seems like it is bug that has been in Fable since a long time but we never encounter it before.

alfonsogarciacaro commented 3 years ago

Can you please try updating to latest Fable.Browser.Event 1.4.5? Hopefully Toth.Elmish.Toast should work again without any changes needed (although you can still use this code if you prefer to use the new generic version of CustomEvent). See https://github.com/fable-compiler/fable-browser/pull/69#issuecomment-859970359

MangelMaxime commented 3 years ago

With the release of Fable.Browser.Event 1.4.5 we can avoid releasing a new version of Thoth.Elmish.Toast it is compatible with current version of the library.

Thank you @PaigeM89 for finding the bug in the Fable.Browser.Event binding :)