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

node/bindnode: improve UX around struct field names #329

Open
mvdan opened this issue Jan 11, 2022 · 0 comments
Open

node/bindnode: improve UX around struct field names #329

mvdan opened this issue Jan 11, 2022 · 0 comments

Comments

@mvdan
Copy link
Contributor

mvdan commented Jan 11, 2022

Right now, bindnode with an IPLD schema and a Go type works for structs as long as:

  • The fields on both sides are in the same order
  • The names are equivalent, e.g. fooBar on IPLD and FooBar on Go (we just uppercase or lowercase the first letter)

That works fine for simple use cases, but it also has some noticeable issues:

  1. It doesn't allow using custom names, such as ID instead of Id or ADL instead of Adl
  2. It doesn't allow using different field orders, which could help for readability, debuggability, or even performance thanks to struct field padding

I propose that we instead do something akin to encoding/json and cbor-gen: the order of fields on the Go side shouldn't matter, and field tags could be used to override the default mapping of names. If a field tag isn't present, we would do a case-insensitive match on the IPLD Schema side, expecting exactly one match.

Another side effect of this change is that, since we don't care about order, we can start ignoring unexported fields on the Go side. This, again, is akin to how encoding/json works.

For example:

type T struct {
    FooBar   string                   // will map to "foobar", "fooBar", "FooBar", or any other such case-insensitive-equal field
    metadata map[string]string        // will be ignored now
    ID       int64 `ipld:"requestID"` // perhaps ID is unambiguous and a better fit for the Go code
}

I'm not set on the field tag being ipld. It could also be something more explicit, like ipldbind. It needs to include the string ipld though, as something like bindnode would be too generic.

cc @warpfork

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

No branches or pull requests

1 participant