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

What to do about VJSON? #585

Closed
dhess opened this issue Jul 28, 2022 · 4 comments
Closed

What to do about VJSON? #585

dhess opened this issue Jul 28, 2022 · 4 comments
Labels
API API issue frontend Affects/is affected by the web frontend question This issue is a question, not a bug or feature request serialization Serialization (e.g., to/from JSON)

Comments

@dhess
Copy link
Member

dhess commented Jul 28, 2022

In this discussion thread, we proposed removing our VJSON type alias, which we originally created to work around some Haskell -> PureScript serialization issues in our Vonnegut prototype:

type VJSON a = CustomJSON '[NoAllNullaryToStringTag] a

However, having briefly revisited this idea this evening, I wonder if we should keep it. I think, at the very least, the StripPrefixAndStartLowercase field label modifier will be useful to eliminate the redundancy imposed by Haskell's need for unambiguous field selectors in Eval.hs. (I experimented with removing these redundant prefixes via DuplicateRecordFields, but unless I'm missing something obvious, this change would make at least our eval tests cumbersome to write due to ambiguous selectors.)

However, we may no longer care about the NoAllNullaryToStringTag modifier, since we originally introduced that for compatibility with purescript-foreign-generic, and TypeScript should handle string enums just fine, as far as I'm aware (assuming Orval can generate nice TypeScript types for them).

Regardless, if we do keep it, we should rename it. I propose something like PrimerJSON.

@dhess dhess added serialization Serialization (e.g., to/from JSON) frontend Affects/is affected by the web frontend question This issue is a question, not a bug or feature request API API issue labels Jul 28, 2022
@dhess
Copy link
Member Author

dhess commented Jul 28, 2022

See #586 for a prospective PR which implements the 'NoAllNullaryToStringTag' and 'PrimerJSON' changes.

@dhess
Copy link
Member Author

dhess commented Jul 28, 2022

For the record, this PR causes no changes in the generated openapi.json for the Primer OpenAPI bindings. However, I suspect that might be due to our very simplified tree API, and that it might affect @georgefst's current work on improving that API.

@georgefst
Copy link
Contributor

As mentioned in #180 (comment) and d559415, any custom JSON encodings present an issue for keeping our OpenAPI spec in sync.

(I experimented with removing these redundant prefixes via DuplicateRecordFields, but unless I'm missing something obvious, this change would make at least our eval tests cumbersome to write due to ambiguous selectors.)

There are options, e.g. like this. The generated field selector functions are the real evil IMO, and are the reason why pre-GHC-9.2 attempts to resolve the duplicate record fields problem haven't gained widespread adoption. As I've mentioned elsewhere, I'd ideally like to see us embrace NoFieldSelectors and use lenses/dots/puns/wildcards for record field access instead.

@dhess
Copy link
Member Author

dhess commented Aug 2, 2022

This was resolved in #599 and #600.

@dhess dhess closed this as completed Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API API issue frontend Affects/is affected by the web frontend question This issue is a question, not a bug or feature request serialization Serialization (e.g., to/from JSON)
Projects
None yet
Development

No branches or pull requests

2 participants