Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breaking change in v2 related to non-F# objects? #20

Closed
matthid opened this issue Feb 19, 2019 · 5 comments · Fixed by #21
Closed

Breaking change in v2 related to non-F# objects? #20

matthid opened this issue Feb 19, 2019 · 5 comments · Fixed by #21

Comments

@matthid
Copy link
Contributor

matthid commented Feb 19, 2019

Description

I have not quite debugged our issue but I feel like the upgrade to fable 2 broke the debugger for us

Repro code

This is a bit hard because basically what we do is embed TypeScript "elmish" components (basically update/view/init written in typescript) into our fable emlish application. This worked fine.

The problem I see is this:
image

image

image

The parameter seems to originate from the following Typescript definition:

type Msg =
    { type: "LanguageMapping", translations: object } |
    { type: "GetInstanceName", name: string | undefined } |
    { type: "TestConnectionAvailable" } |
    { type: "ConnectionAvailable", available: boolean } |
    ... several other cases left out ...;

Expected and actual results

Expected: Debugger can work with non-F# objects
Actual: Debugger fails

Related information

  • elmish version: Fable.Elmish (2.0.3)
  • fable-compiler version: "fable-compiler": { "version": "2.1.10",
  • fable-core version: Fable.Core (2.0.3)
  • Operating system: Win 7 (I know but I'm pretty sure it is not related to the OS)
@MangelMaxime
Copy link
Member

The debugger is now using Thoth.Json as Fable 2 removed toJson and ofJson.

Because the debugger is also using version 2 of Thoth.Json if it doesn't understand the type it will generate an error.

Thoth.Json isn't able to serialize/deserialize an unknown type. However, in version 3 of Thoth.Json, we added the possibility to extends the known types by Thoth.Json docs.

I don't know if this is possible to extend it in order to support non F# types. Perhaps, by defining a binding for the type it can be possible or perhaps I need to allow adding non F# type support to Thoth.Json with the limitation that the type is identifiable using typeof or a test function. In the case of your type, the test function would check for type property for example.

I didn't release Thoth.Json v3 yet because I didn't get a lot of feedback on it. And wanted to finish the docs upgrade.

As a quick fix, I think you will need to copy the debugger function and use Thoth.Json v3 to check if you can deserialize TypeScript types.

@matthid
Copy link
Contributor Author

matthid commented Feb 19, 2019

Hm, maybe then Thoth is just no applicable for us and we need a way to opt-out and use normal Js-object -> Json serialization?

I can try to find a workaround where I use Thoth for fable components and use "simple" js serialization for typescript components

@et1975
Copy link
Member

et1975 commented Feb 19, 2019 via email

matthid added a commit to matthid/debugger that referenced this issue Feb 22, 2019
matthid added a commit to matthid/debugger that referenced this issue Feb 22, 2019
fixes elmish#20

Actually, I think this should have created problems for anyone not using F#-unions.
et1975 pushed a commit that referenced this issue Feb 22, 2019
fixes #20

Actually, I think this should have created problems for anyone not using F#-unions.
et1975 pushed a commit that referenced this issue Feb 22, 2019
@matthid
Copy link
Contributor Author

matthid commented Feb 22, 2019

@MangelMaxime apparently this wasn't an issue with Thoth after all. There was just code added which assumed that the type is a F#-DU which it obviously isn't in my scenario...

@MangelMaxime
Copy link
Member

Good catch :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants