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

Add tests to document behavior of Codec.nullableField and Code.maybeField #17

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

marc136
Copy link

@marc136 marc136 commented Jun 2, 2023

Thank you @dillonkearns for creating elm-ts-interop and also this nice package.

I created this PR to document a behavior that was unexpected to me:
I had expected that nullableField returns an Err when the field exists but the decoder fails. But it returns an Ok Nothing.
The generated TS type seems like it should not hide the error message { f : number | null } when passing { f: "string"}.

I'm not sure if this is the intended behavior or a bug, so I first wanted to add the tests and then ask for your opinion.

Would you be open to an addition of a codec with this documentation? Or changing the behavior of nullableField?

  1. If the value is null the decoder will return Nothing.
  2. If the value has the wrong type, the encoder will error out.
  3. If the value is Nothing then the resulting object will contain the field with a null value.

It seems related to miniBill/elm-codec#9 (comment) and it might also very well be the case that you are not interested in changing the behavior because it breaks backwards compatibility or because you want to stay close to minibills codec implementation.

Both return a `Maybe value`, and if the decoder fails, they return an
`Ok Nothing`.
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 this pull request may close these issues.

1 participant