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

Is null a vaild schema type or not ? #266

Open
Gozala opened this issue Jan 17, 2023 · 7 comments
Open

Is null a vaild schema type or not ? #266

Gozala opened this issue Jan 17, 2023 · 7 comments
Assignees

Comments

@Gozala
Copy link

Gozala commented Jan 17, 2023

I'm bit confused about null, in the schema schema it's not defined as a kind

type RepresentationKind enum {
| Bool ("bool")
| String ("string")
| Bytes ("bytes")
| Int ("int")
| Float ("float")
| Map ("map")
| List ("list")
| Link ("link")
}

Nor a type

type TypeKind enum {
| Bool ("bool")
| String ("string")
| Bytes ("bytes")
| Int ("int")
| Float ("float")
| Map ("map")
| List ("list")
| Link ("link")
| Union ("union")
| Struct ("struct")
| Enum ("enum")
| Unit ("unit")
| Any ("any")
}

Yet there is test that seems to assume null is a kind

schema: |
type Nothing null
expected: |
{
"types": {
"Nothing": {
"kind": "null"
}
}
}

So which one is correct and which on isn't ?

It also appears that type defined in prelude type Null unit representation null does not compile with js-toolchain, although not sure if it's a bug in js-toolchain or perhaps prelude is out of date ?

@rvagg
Copy link
Member

rvagg commented Jan 23, 2023

Yeah, this is a tricky one. You can see that I've stumbled on this myself here: https://github.com/ipld/js-ipld-schema/blob/9677e35c7d48feb789d6e09f5a7a7d3331c3d3b8/schema-schema.ts#L29

In my opinion it should be defined as a typekind. It should be usable everywhere a scalar is usable. But it has a cardinality of 1, unlike everything else.

But, I think the reason Eric left it out are twofold:

  1. nullable is a thing and almost everywhere you want a null you should be able to use a nullable. Unfortunately it's not 100% of the time.
  2. unit should solve the rest of the % not covered by nullable and it has the benefit of having different representations. The cases where you're not doing a nullable field, you're using it either as a placeholder or a signal entry.

type Null unit representation null should be the appropriate replacement with the current Schema spec.

Regarding the JS tooling: you're correct that it doesn't compile, unit is one of the (maybe the) most recent changes to the the schema language and it just hasn't caught up. I don't believe there's even cursory support for it in Go. The closest thing to "support", aside from being in the schema-schema, is that it's in the TypeScript definitions in js-ipld-schema!

So it's a feature that's just waiting for someone (like you!) to show up with a usecase where it's really annoying to not have it and then implement it. Would you like to have a go? I believe that the parser in js-ipld-schema should be fairly clear, but I've not had anyone else contribute to it so I may be wrong about that! I'd love feedback on the process of adding features.

@rvagg
Copy link
Member

rvagg commented Jan 23, 2023

Oh, and re the ipld/specs/schemas/tests/null.yml-old test .. I think that one actually works in JS? Maybe. At least it used to. All those fixtures come out of my JS implementation which allowed null as a typekind. I reckon I was stubborn enough to keep that in there but I haven't checked. Any of those tests with -old are not currently used by the Go test suite and may not even be used by the JS suite. So don't take them as a source of truth. They're a source of my personal historical truth which may or may not reflect anyone else's, or even my own present truth!

@Gozala
Copy link
Author

Gozala commented Jan 24, 2023

Oh, and re the ipld/specs/schemas/tests/null.yml-old test .. I think that one actually works in JS? Maybe. At least it used to.

Yeah this works

But the one in prelude that uses unit does not as you've said.

https://observablehq.com/d/9cbd647974be5067

@Gozala
Copy link
Author

Gozala commented Jan 24, 2023

So it's a feature that's just waiting for someone (like you!) to show up with a usecase where it's really annoying to not have it and then implement it. Would you like to have a go?

I have side project around IPLD schemas that I'm hacking on spare time, which I'd love to share once I get around to getting it usable state which attempts to:

  1. Add support for generics per Support for generics in schema #248
  2. Support inline type defs as per Extend IPLD schema to support inline types #262
  3. Notion of functions IPLD schema for function #263
  4. Removes named types in favor of CID based addressing (This is my main gripe with schemas today)
  5. Uses lisp notation, because custom syntax doesn't seem to add any value just overhead IMO (anyone else can add alternative syntax if they wish).
  6. Plans to support compiling down to the standard schema

I'm not sure where I'll land with that attempt, but I'm sure it's good field study that my result in retrofitting some ideas into current tool chains.

@Gozala
Copy link
Author

Gozala commented Jan 24, 2023

For what it's worth I end up defining Nullables as type Nullable<T> union { | Null null | Value T } representation kinded which in structs, gets normalized into just nullable filed with type T.

While I do appreciate what Erik was trying to do there, yet I find it ended up in strange place where null is neither first class thing nor it is a field modifier. It's kind of tries to be later, yet fails and consequently ends up been neither. Here is the illustration of where it leaks:

type Message struct {
  message nullable NullableString
}

type NullableString union {
  | Null null
  | String string
} representation kinded

P.S.: I kind of wish null or nullable was not even a thing apart from representation strategy of unit

@Gozala
Copy link
Author

Gozala commented Jan 24, 2023

In my opinion it should be defined as a typekind. It should be usable everywhere a scalar is usable. But it has a cardinality of 1, unlike everything else.

Doesn't int has cardinality of 1 as well ? Or do you meant that nullable would not affect cardinality of the null ?

@rvagg
Copy link
Member

rvagg commented Jan 30, 2023

re your point about nullable and a union involving Null - you're right, and I feel similarly

I kind of wish null or nullable was not even a thing apart from representation strategy of unit

Keeping in mind that unit is a later addition. But also, I think the design choice here was one of usability - nullable is super common in practice. If you had to do a union every time you wanted one of these then you'd be very verbose. Perhaps having generics like you've outlined would be nice and solve much of this, but we'd have to resolve that problem of where generics live - if they are just sugar to compile to a more verbose DSL then that's going to get really ugly really quick. If they live inside the DMT then that may be more palatable; but that's where the hard work is.

For now, nullable is a nice usability feature for a common form of data, but creates a weird edge where you want to use Null as a thing. But most of that should be solved by having unit support - which we need in JS still I suppose.

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

No branches or pull requests

2 participants