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

feat(gen): const from default #7

Merged
merged 8 commits into from
Aug 17, 2021
Merged

feat(gen): const from default #7

merged 8 commits into from
Aug 17, 2021

Conversation

sh0rez
Copy link
Member

@sh0rez sh0rez commented Mar 11, 2021

Generates a matching const for each type definition that holds the defaults for the type based on information from CUE defaults.

Status: Useful (I guess)

CUE TS
package cuetsy

AType: "foo" | "bar" | *"baz" @cuetsy(targetType="type")

Foo: {
  Bar: string | *"ohai"
  Baz: int | *4
  C: AType
} @cuetsy(targetType="interface")
export type AType = 'foo' | 'bar' | 'baz';
export const aTypeDefault: AType = 'baz'
export interface Foo {
  Bar: string;
  Baz: number;
  C: AType;
}
export const fooDefault: Foo = {
  Bar: 'ohai',
  Baz: 4,
  C: aTypeDefault,
}
package cuetsy

OrEnum: "foo" | "bar" | *"baz" @cuetsy(targetType="enum")

StructEnum: {
   Walla: "laadeedaa" @cuetsy(enumDefault)
   run: "OMG"
} @cuetsy(targetType="enum")
export enum OrEnum {
  Bar = 'bar',
  Baz = 'baz',
  Foo = 'foo',
}
export const orEnumDefault: OrEnum = OrEnum.Baz
export enum StructEnum {
  Walla = 'laadeedaa',
  run = 'OMG',
}
export const structEnumDefault: StructEnum = StructEnum.Walla

Generates a matching const for each type definition that holds the
defaults fot the type based on information from CUE defaults.
@ryantxu
Copy link
Member

ryantxu commented Mar 12, 2021

excellent! 2 questions:

  1. can generating defaults be optional? We will only want it for some types of objects (namely, panelOptions and panelFieldConfig) Looks like it already is 🎉
  2. can the default name be lowercase?
    export const fooDefault: Foo ...

Hard-disables defaults generation for list fields of structs.

We don't allow `[...number] | *[1,2]` anyways, so this does not take
anythinga way, but prevents `[]` all over the place
@sh0rez sh0rez marked this pull request as ready for review March 12, 2021 12:04
@sh0rez sh0rez requested a review from sdboyer March 12, 2021 12:05
@sh0rez sh0rez added component/generator kind/feature New feature or request labels Mar 12, 2021
@sh0rez
Copy link
Member Author

sh0rez commented Mar 12, 2021

@sdboyer @ryantxu afaict it mostly does what it is supposed to do. There are for sure rough edges, but it should be good enough for a first round of review

@ryantxu
Copy link
Member

ryantxu commented Mar 12, 2021

The results here look great!

If it is possible to optionally tag interfaces that we want to generate a default that would be best -- for most types we do not want a default and it will just create noise bloat. It is only the final full options objects where we want. Perhaps:

} @cuetsy(targetType="interface", generateDefaults=true)

@sdboyer
Copy link
Contributor

sdboyer commented Aug 17, 2021

So, months later...

i'm trying to recall why it is that we thought struct representations of enums were a good idea. I think it was because we were looking for a way to encode arbitrary member names easily. But it's just wrong. CUE structs are plainly, not analogous to a TS enum. In a system (like we have in Grafana), using the CUE struct representation directly in some larger CUE structure means that you'll end up with a struct type on the CUE side, and an enum on the TS side. Not what we want.

The rest of this looks good, but i think that part's just not what we need. i introduced an alternative pattern that seems like it should give us the same control (even if it's a bit awkward), while having a CUE type (simple disjunct value) which is harmonious with the TS it will become.

@sdboyer
Copy link
Contributor

sdboyer commented Aug 17, 2021

i'm trying to recall why it is that we thought struct representations of enums were a good idea.

Well, it was literally the first issue - #1. Not the fault of this PR. So i'm gonna merge this one as-is, then we can pull that behavior out.

@sdboyer sdboyer merged commit e00c497 into main Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants