feat: Switch packages for typescript generation code #1196

Emyrk · 2022-04-27T16:30:21Z

Redo typescript autogen

Ok, this got a lot more complicated, but it handles all the cases. I didn't add a unit test, because we will just use the generated types as our testing. If a type is wrong, hopefully the FE team will complain and we fix it.

Supports Go types
- Basics (string/int/etc)
- Maps
- Slices
- Enums
- Pointers
- External Types (uses any atm)
  - Some custom external types are hardcoded in (eg: time.Time)

Features

Ignore types

http://github.com/coder/coder/blob/071026deca83f558f9c5be3c70f90cda0105aa82/codersdk/client.go#L29-L29

Choose typescript values

type Example struct {
   FieldUseString CustomType `json:"field_string" typescript:"string"`
   Ignore *http.Client `json:"-" typescript:"-"`
}

Issues

How do we handle time.Time?

…escript_gen

codecov · 2022-04-28T14:52:02Z

Codecov Report

Merging #1196 (0d37eeb) into main (816441e) will decrease coverage by 9.36%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #1196      +/-   ##
==========================================
- Coverage   66.00%   56.64%   -9.37%     
==========================================
  Files         265      265              
  Lines       16751    16937     +186     
  Branches      157      157              
==========================================
- Hits        11057     9594    -1463     
- Misses       4533     6280    +1747     
+ Partials     1161     1063      -98

Flag	Coverage Δ
unittest-go-macos-latest	`52.88% <0.00%> (-0.57%)`	⬇️
unittest-go-postgres-	`?`
unittest-go-ubuntu-latest	`55.37% <0.00%> (-0.65%)`	⬇️
unittest-go-windows-2022	`52.46% <0.00%> (-0.60%)`	⬇️
unittest-js	`66.50% <ø> (ø)`

Impacted Files	Coverage Δ
codersdk/client.go	`59.74% <ø> (ø)`
scripts/apitypings/main.go	`0.00% <0.00%> (ø)`
coderd/database/queries.sql.go	`0.00% <0.00%> (-81.13%)`	⬇️
coderd/devtunnel/tunnel.go	`0.00% <0.00%> (-79.67%)`	⬇️
coderd/database/pubsub.go	`0.00% <0.00%> (-77.78%)`	⬇️
coderd/database/db.go	`0.00% <0.00%> (-55.18%)`	⬇️
coderd/database/migrate.go	`0.00% <0.00%> (-45.00%)`	⬇️
peerbroker/dial.go	`77.04% <0.00%> (-6.56%)`	⬇️
coderd/coderdtest/coderdtest.go	`93.83% <0.00%> (-5.05%)`	⬇️
cli/server.go	`54.97% <0.00%> (-3.94%)`	⬇️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 816441e...0d37eeb. Read the comment docs.

greyscaled · 2022-04-28T16:05:24Z

scripts/apitypings/README.md

@@ -0,0 +1,39 @@
+# APITypings


praise: Wonderful documentation!

greyscaled · 2022-04-28T16:06:15Z

site/src/api/typesGenerated.ts

+  readonly email: string
+  readonly username: string
+  readonly password: string
+  readonly organization_id: string


👍 nice, there it is

cc @presleyp 🎉

Pretty much any non-primitive (basic type) wasn't included before. uuid is a named array type of byte.

All now handled. The unhandled cases in the past omitted the field, now they use any and leave a comment.

coder/site/src/api/typesGenerated.ts

Lines 213 to 215 in 3865f36

// Named type "github.com/coder/coder/coderd/database.ParameterValue" unknown, using "any"

// eslint-disable-next-line @typescript-eslint/no-explicit-any

readonly ParameterValue: any

I also comment where I use some "intelligence" about external types.

coder/site/src/api/typesGenerated.ts

Lines 227 to 228 in 3865f36

// This is likely an enum in an external package ("github.com/coder/coder/coderd/database.ParameterSourceScheme")

readonly default_source_scheme: string

One thing to note is nothing here is set in stone, and if the FE team thinks a type is wrong, we can fix it. The current implementation comments all guesses though, so you know where things are maybe a bit ambiguous, or could be improved.

f0ssel

Overall great improvement. Glad we can even add doc comments for types and fields in the future with this route. Only real unknowns are the byte arrays but we can figure that out.

f0ssel · 2022-04-28T16:08:37Z

scripts/apitypings/main.go

+		case bs.Info()&types.IsBoolean > 0:
+			return TypescriptType{ValueType: "boolean"}, nil
+		case bs.Kind() == types.Byte:
+			// TODO: @emyrk What is a byte for typescript? A string? A uint8?


Idk if anyone actually uses this... but https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/ArrayBuffer exists.

If we add []byte to a type, we can adjust as needed. For now, I just use a number for a byte, and string for []byte 🤷.

TIL about that

I was guessing these types should accept the golang marshal json. So this will at least work in the sense the 2 types can be communicated over json.

f0ssel · 2022-04-28T16:08:42Z

scripts/apitypings/main.go

 )

+// TODO: Handle httpapi.Response and other types


We should focus on making this script work perfectly for what we already plan to do in codersdk. If this is relevant with that in mind, you should probably make this more specific to the code that handles this.

I don't know if we need it. This is really to match Go types, but the general error/response type is not bad to implement once in TS for all endpoints that use it.

I don't want to run this code on another package, and type inferencing across packages is only possible to an extent.

I removed the TODO, but the basic thing is that external types are not currently handled. The only way to handle them better, is to run this code on those packages as well. Obviously then we want to allowlist types, vs the current deny list approach.

Here is the example of external types:
http://github.com/coder/coder/blob/3865f36c4bf8c2ddfc1c9081cf7bbf1a881675f6/codersdk/templateversions.go#L27-L30

They use database types that include enums. I would like to have a conversation about how to handle these, as we can handle them. Just a matter of "how"

f0ssel · 2022-04-28T16:10:25Z

scripts/apitypings/main.go

+			return TypescriptType{ValueType: bs.Name()}, nil
+		}
+	case *types.Struct:
+		// This handles anonymous structs. This should never happen really.


Maybe we just error and tell the user not to do that? I could go either way, but idk if it's worth explicitly supporting.

I like my current approach of just leaving a comment that it should be fixed and accept any.

I was afraid to error, as it's kinda a pain when the error blocks your work. The any type is valid typescript, so it still will work from a compile time perspective.

I do see the benefit of an error though as it does force the change up front. I just see this code as more a way for the FE and BE to agree on api types, rather than a strict document that has no flexibility.

I can cave on this if you feel strongly otherwise. In a month or two, I'm happy to make this more strict as we know more what we want.

f0ssel · 2022-04-28T16:11:37Z

scripts/apitypings/main.go

+		// When type checking here, just use the string. You can cast it
+		// to a types.Basic and get the kind if you want too :shrug:
+		case arr.Elem().String() == "byte":
+			// All byte arrays are strings on the typescript.


@vapurrmaid could you weigh in here?

Yea, really curious here. The json marshal is a string, which is why I chose it. It's base64 encoded. Not ideal still?

https://go.dev/play/p/mv6bH0b9g4N

f0ssel · 2022-04-28T16:13:08Z

site/src/api/typesGenerated.ts

  readonly name: string
+  // This is likely an enum in an external package ("github.com/coder/coder/coderd/database.ParameterSourceScheme")


Nice stop gap 👍

f0ssel · 2022-04-28T16:13:38Z

site/src/api/typesGenerated.ts

-// From codersdk/provisionerdaemons.go:26:6.
-export type ProvisionerJobStatus = "pending" | "running" | "succeeded" | "canceling" | "canceled" | "failed"
+// From codersdk/provisionerdaemons.go:26:6
+export type ProvisionerJobStatus = "canceled" | "canceling" | "failed" | "pending" | "running" | "succeeded"


ty for sorting, will prevent flakes

f0ssel · 2022-04-28T16:16:53Z

If we don't see us returning byte arrays from codersdk in the near future we can leave what is here and kick that can down the road. My guess would be an array of ints but idk for sure.

presleyp

Yay, thanks!

Emyrk · 2022-04-28T16:32:39Z

scripts/apitypings/main.go

+		switch n.String() {
+		case "net/url.URL":
+			return TypescriptType{ValueType: "string"}, nil
+		case "time.Time":
+			// We really should come up with a standard for time.
+			return TypescriptType{ValueType: "string"}, nil
+		case "database/sql.NullTime":
+			return TypescriptType{ValueType: "string", Optional: true}, nil
+		case "github.com/google/uuid.NullUUID":
+			return TypescriptType{ValueType: "string", Optional: true}, nil
+		case "github.com/google/uuid.UUID":
+			return TypescriptType{ValueType: "string"}, nil
 		}


Would be dope if this was a yaml config instead.

* feat: Switch packages for typescript generation code Supports a larger set of types - [x] Basics (string/int/etc) - [x] Maps - [x] Slices - [x] Enums - [x] Pointers

Emyrk added 11 commits April 27, 2022 11:28

feat: Switch packages for typescript generation code

ee71acd

Add enum functionality

b0b17d0

Handle reused types

28c2811

Add comment

710b427

Allow ignoring types for autogen

071026d

Fix ignore, add optional fields

2741efa

Add map support

c62feeb

Handle unknown types better

462ebeb

Add comments, run gen

21d1687

Add basic readme

9a3d7e3

Merge remote-tracking branch 'origin/main' into stevenmasley/redo_typ…

1afaa5d

…escript_gen

Emyrk marked this pull request as ready for review April 28, 2022 14:46

Emyrk requested a review from a team as a code owner April 28, 2022 14:46

Reorder imports

7cfba41

Emyrk added 4 commits April 28, 2022 09:54

Sort all output

0127e6d

Add 'DO NOT EDIT' to the top

0d943f1

Go linting

2379c21

Fix typescript linter

3865f36

greyscaled mentioned this pull request Apr 28, 2022

feat: Create user page #1197

Merged

Emyrk requested review from kylecarbs, f0ssel, BrunoQuaresma and greyscaled April 28, 2022 16:03

greyscaled approved these changes Apr 28, 2022

View reviewed changes

greyscaled requested a review from presleyp April 28, 2022 16:06

presleyp mentioned this pull request Apr 28, 2022

Switch to using generated types #1210

Closed

f0ssel approved these changes Apr 28, 2022

View reviewed changes

presleyp approved these changes Apr 28, 2022

View reviewed changes

Use correct array size for allocation

176b481

Emyrk commented Apr 28, 2022

View reviewed changes

Update readme

0d37eeb

Emyrk enabled auto-merge (squash) April 28, 2022 16:40

Emyrk merged commit e330dc1 into main Apr 28, 2022

Emyrk deleted the stevenmasley/redo_typescript_gen branch April 28, 2022 16:59

misskniss added the V2 BETA label May 15, 2022

misskniss added this to the V2 Beta milestone May 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Switch packages for typescript generation code #1196

feat: Switch packages for typescript generation code #1196

Emyrk commented Apr 27, 2022 •

edited

Loading

codecov bot commented Apr 28, 2022 •

edited

Loading

greyscaled Apr 28, 2022

greyscaled Apr 28, 2022

greyscaled Apr 28, 2022

Emyrk Apr 28, 2022

f0ssel left a comment

f0ssel Apr 28, 2022

Emyrk Apr 28, 2022

Emyrk Apr 28, 2022

f0ssel Apr 28, 2022

Emyrk Apr 28, 2022

f0ssel Apr 28, 2022

Emyrk Apr 28, 2022

Emyrk Apr 28, 2022 •

edited

Loading

f0ssel Apr 28, 2022

Emyrk Apr 28, 2022 •

edited

Loading

f0ssel Apr 28, 2022

f0ssel Apr 28, 2022

f0ssel commented Apr 28, 2022

presleyp left a comment

Emyrk Apr 28, 2022

	// Named type "github.com/coder/coder/coderd/database.ParameterValue" unknown, using "any"
	// eslint-disable-next-line @typescript-eslint/no-explicit-any
	readonly ParameterValue: any

	// This is likely an enum in an external package ("github.com/coder/coder/coderd/database.ParameterSourceScheme")
	readonly default_source_scheme: string

		readonly name: string
		// This is likely an enum in an external package ("github.com/coder/coder/coderd/database.ParameterSourceScheme")

feat: Switch packages for typescript generation code #1196

feat: Switch packages for typescript generation code #1196

Conversation

Emyrk commented Apr 27, 2022 • edited Loading

Redo typescript autogen

Features

Ignore types

Choose typescript values

Issues

codecov bot commented Apr 28, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

f0ssel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Emyrk Apr 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Emyrk Apr 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

f0ssel commented Apr 28, 2022

presleyp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Emyrk commented Apr 27, 2022 •

edited

Loading

codecov bot commented Apr 28, 2022 •

edited

Loading

Emyrk Apr 28, 2022 •

edited

Loading

Emyrk Apr 28, 2022 •

edited

Loading