-
Notifications
You must be signed in to change notification settings - Fork 322
[playground] Add/enable protobuf kiosk sample in the playground #1863
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
[playground] Add/enable protobuf kiosk sample in the playground #1863
Conversation
|
Changes in this PR will be published to the following url to try(check status of TypeSpec Pull Request Try It pipeline for publish status): Website: https://cadlwebsite.z1.web.core.windows.net/prs/1863/ |
| const [selected, setSelected] = useState<"raw" | "swagger-ui">("raw"); | ||
| const options = [ | ||
| { label: "OpenAPI", value: "raw" }, | ||
| { label: "Output Files", value: "raw" }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hhm, thats another issue didn' think of, we don't really want that dropdown to be showing for non openapi output
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, you get a kind of ugly warning about missing the OpenAPI version number if you set it to Swagger UI when the protobuf emitter is selected...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we should have yet another config to load a plugin(swagger view) for certain emitters. become out of scope of your pr though
| "REST framework": "samples/rest.tsp", | ||
| "API versioning": { | ||
| fileName: "samples/versioning.tsp", | ||
| preferredEmitter: "@typespec/openapi3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it not be better to have this as an array of emitters that are compatible with this sample?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this PR I'm hoping to do the very minimum thing to light up typespec/protobuf in the existing playground so that we can share it. If we want to rework the playground a little bit to have an emitter -> sample flow then we should do that separately IMO and just let this be a dead simple autoselect for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure lets merge that and update after, can you just make a PR updating typespec-azure with the new option too
CC @bterlson @timotheeguerin
This PR adds support for the Protobuf emitter to the TypeSpec playground. It adds the Protobuf library/emitter to the list of libraries and makes a
kiosk.tspsample available. This sample is based on one of Google's official examples for Protobuf. Since this is the first sample in the playground that isn't intended to be used with the OpenAPI 3 emitter, I made a couple of changes:preferredEmitter. When a sample is selected, the preferred emitter is automatically selected as well. If the sample has not preferred emitter, then the default emitter is used, but I addedpreferredEmitterto all the current samples.index.tsmodule.os.EOLin the Protobuf library since the playground does not polyfill any surface ofos.google.protobuf.LatLng) that is used by the Kiosk sample.I also fixed a little bug where the emitter dropdown says "Select sample..." instead of "Select emitter..."