VSCode extension: add openapi3 preview #6015
Conversation
# Conflicts: # packages/typespec-vscode/package.json # packages/typespec-vscode/src/extension.ts # packages/typespec-vscode/src/types.ts # pnpm-lock.yaml
# Conflicts: # pnpm-lock.yaml
# Conflicts: # pnpm-lock.yaml
archerzz
requested review from
RodgeFu,
bterlson,
markcowl,
allenjzhang,
timotheeguerin,
chrisradek and
AlitzelMendez
as code owners
|
I have to copy |
|
All changed packages have been documented.
|
|
You can try these changes here
|
|
https://github.com/arjun-g/vs-swagger-viewer is not suitable as a dependent extension for previewing swagger, because it only exposes a load method, which doesn't fulfill our requirement:
|
archerzz
requested review from
iscai-msft,
tadelesh,
srnagar,
weidongxu-microsoft,
haolingdong-msft,
XiaofeiCao,
m-nash,
JoshLove-msft and
jorgerangel-msft
as code owners
|
List of changes since last review:
|
|
List of changes since last review:
|
timotheeguerin
changed the title
| @@ -27,6 +28,24 @@ export async function isDirectory(path: string) { | |||
| } | |||
| } | |||
|
|
|||
| export async function createTempDir(): Promise<string | undefined> { | |||
There was a problem hiding this comment.
node has mkdtemp which does that too
| return; | ||
| } | ||
|
|
||
| const mainTspFile = selectedFile.endsWith("main.tsp") ? selectedFile : await getMainTspFile(); |
There was a problem hiding this comment.
this is the same issue as #6062, if the user asked for a preview of a file it should respect that and not try to fetch a main.tsp
There was a problem hiding this comment.
when I discussed this feature with Rodge, we thought we should show the full open api 3 spec v.s. just the selected spec. Sometimes, showing the selected spec may be meaningless. For example, client.tsp is for client codes, so it should not have impact on the Open API 3 output which is for service API.
@RodgeFu Your thought?
There was a problem hiding this comment.
Yeah but it's not up to you to decide that for the user. Typespec compile takes any file as entry point it just default to a file called main.tsp when you target a directory compile .
Client.tsp is not the only use case. At the very least there should be a choice the user can make.
There was a problem hiding this comment.
I think this is about how the files in project can be compiled properly. I believe in most case, starting compilation from "main.tsp" is still the right way to go and simply using the file user selected as entrypoint is likely not to compile the project as expected. So I think we should keep the "main.tsp" as the default behavior while providing some way for people to have the choice when needed.
Some options I can think about:
- provide a settings for user to set entrypoint file when needed
- adding a quickpick step for people to select main.tsp (default) or the selected file when user clicks preview
Actually the first one feels like something should be in tspconfig.yaml instead of vscode. @timotheeguerin , do we have any plan for it there?
open to any better ideas.
There was a problem hiding this comment.
right now there is if there is a exports or tspMain(deprecated) in package.json but nothing to have sub project per package. Having many project per package is still quite a common use case(whole of the specs repos)
Maybe we should investigate also providing an entrypoint from the config but the way it works today is the other way around it resolve a config for an entrypoint which many can share
There was a problem hiding this comment.
yeah, I think it will be useful if user can define the entrypoint in the config when people using tspconfig.yaml per project instead of package.json like the spec repo.
As for this PR, considering we are already able to config the entrypoint in package.json for user to customize now, I think we can try to check package.json file with tspMain/exports configured first. if found, then start compilation from there (just pass the folder and cli should be able to handle it). Otherwise, fallback to trying to find main.tsp. Later on, when we support entrypoint in tspconfig.yaml, we can add that part.
And about the exports and tspMain in package.json, I checked the code to resolve entrypoint for compile and found it only checks tspMain in the package.json but nothing about exports. Also I can only find exports and tspMain are mentioned for Package import in the doc. Could you help to provide more detail about the exports as entrypoint of compile? (
There was a problem hiding this comment.
you are right it doesn't do that now, but this is a bug, it only resolve exports when doing imports or emitter names.
This is a bug because the main purpose of this is to be able to build your library (where you might just have exports now and this means you cannot do tsp compile . in your library package)
| } | ||
|
|
||
| async function getOutputFolder(mainTspFile: string): Promise<string | undefined> { | ||
| let tmpFolder = openApi3TempFolders.get(mainTspFile); |
There was a problem hiding this comment.
when does caching become useful here?
There was a problem hiding this comment.
- When a tsp file is changed, the file watcher will re-generate open api 3 output and reload the preview panel. At that time, if the corresponding temp folder is already created, it will re-use that folder.
- When the preview panel is closed and opened again, the same logic will be triggered.
There was a problem hiding this comment.
Do you clear the dir in between, if the user goes change the name of services or version it could create ghost files otherwise
vsce packagenot supporting pnpm, I have to copyswagger-ui-distlib files into our source folder. So, add those files into ignore list of lint and format.resolve #4858