Skip to content

Fix types generated for WorkspaceResource "type" field #3767

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.

Sign up for GitHub

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

Jump to bottom
Closed
3 tasks
jsjoeio opened this issue Aug 30, 2022 · 1 comment
Closed
3 tasks

Fix types generated for WorkspaceResource "type" field #3767

jsjoeio opened this issue Aug 30, 2022 · 1 comment
Assignees
@jsjoeio
Labels
api Area: HTTP API

Comments

@jsjoeio
Copy link
Contributor

jsjoeio commented Aug 30, 2022 •
edited
Loading

Problem

While working on #3754, I realized our types are a bit loose. On the frontend, we have a small set of possibilities listed here:

https://github.com/coder/coder/blob/main/site/src/components/Resources/ResourceAvatar.tsx#L16-L25

If this is the case, our generated types should use a union of strings instead of string. This will lead to safer code

Solution

Modify https://github.com/coder/coder/blob/main/codersdk/workspaceresources.go#L28 to be a limited subset of strings similar to what we do for BuildReason here:

https://github.com/coder/coder/blob/main/codersdk/workspacebuilds.go#L22-L34

Checklist

  • Modify Go types
  • Generate new TS types
  • Use on frontend
@jsjoeio jsjoeio added the api Area: HTTP API label Aug 30, 2022
@jsjoeio jsjoeio self-assigned this Aug 30, 2022
@jsjoeio jsjoeio mentioned this issue Aug 30, 2022
Merged
@jsjoeio
Copy link
Contributor Author

jsjoeio commented Aug 31, 2022

I spoke to @BrunoQuaresma offline about this:

It is hard to predict those types tho. Because it can be anything.
I think we are not able to map all the Terraform resource types and even if we could, it can be a hundreds or thousands
What I made in this case was to map the most used ones based on user usage and we probably should update it time to time

In that case, it doesn't make sense to change them on the backend so closing this for now.

@jsjoeio jsjoeio closed this as not planned Won't fix, can't repro, duplicate, stale Aug 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jsjoeio jsjoeio

Labels
api Area: HTTP API
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@jsjoeio