-
Notifications
You must be signed in to change notification settings - Fork 8
Avoid conditionals for defining tagged unions. #151
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
base: main
Are you sure you want to change the base?
Conversation
Thanks for putting this together @RaoulSchaffranek! It's funny you should bring this up; originally the schemas were just tagged unions, but error reporting with The trouble with But yeah, it's a really big issue... tooling support for JSON Schema is not caught up with draft 2020-12 yet :(. I have high confidence that eventually this will happen... I've watched JSON Schema tooling support lag behind the spec for probably a decade now, and it does slowly chug along. Anyway, what if there's a different approach? With the way I've done it here in ethdebug/format, I've chosen several different schema design patterns (as you've noted) based on my own perceived ergonomics... but I'll note that my requirement for myself, in doing so, has been to make sure I've formalized the pattern enough for it to be legible on the website. Here's what I mean: ![]() For instance, here with the ethdebug/format/pointer/region schema, as you've identified, I use To summarize this point about use of various conventions: good observation... yes, I do make liberal use of various JSON Schema design conventions. I am keeping track of them, but they are not formalized or even enumerated. Sorry for the long story, but my point is this: what if this format used modern JSON Schema and just published a properly formalized list of conventions used, to aid in things like automated code generation? I'm even thinking that such an artifact would be useful in machine translation from JSON Schema draft 2020-12 to draft-7 or whatever your tooling is prepared to handle. |
Side note: I think with ethdebug/format, automatic code generation is going to be more trouble than it's worth. I tried this and drove myself mad, and then switched to writing types and type guards by hand (e.g. pointer schema types in TypeScript). Of course, my telling people "I advise against doing automatic code generation with ethdebug/format schemas" doesn't change the issue here, which I appreciate your bringing to my attention (I've been entirely focused on schema validation quality as the priority). |
(BTW my guess is that the preview job is probably failing because you're on a fork) |
|
Avoid conditionals for defining tagged unions.
This PR simplifies the schema structure of the region and collection listings by replacing a large
if
-based conditional block with a more conciseoneOf
declaration. The resulting schemas should be logically equivalent but offer better compatibility with tooling.Benefits of using
oneOf
:oneOf
.oneOf
has been around longer and is better supported across the ecosystem. Validation and code generation libraries (e.g., Python's datamodel-code-generator) have built-in support foroneOf
, but haven't yet caught up withif
-blocks.if/then
, which often requires complex conditional typing,oneOf
maps more directly to union types or class hierarchies.Background
In our case, we use datamodel-code-generator to generate Python validators from the JSON Schema. This tool supports
oneOf
but does not yet supportif/then
, making the refactor necessary for proper code generation.Additionally, this PR changes an instance of
allOf: true
toallOf: {}
. These are logically equivalent, but the former is a newer convention not yet supported by all tools, includingdatamodel-code-generator
.Additional considerations
The grouping and expression schemas currently discriminate between their variants based on the presence of specific properties. In contrast, the region schema uses the value of the
location
field to distinguish its variants. This inconsistency might be worth addressing.