-
-
Notifications
You must be signed in to change notification settings - Fork 436
Add options for import file extensions and type-only imports #1830
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
Open
Upgrade220
wants to merge
3
commits into
acacode:main
Choose a base branch
from
Upgrade220:feature/import-file-extension-and-type-only-imports
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| --- | ||
| "swagger-typescript-api": minor | ||
| --- | ||
|
|
||
| Add `importFileExtension` and `typeOnlyImports` options | ||
|
|
||
| `importFileExtension` (`""` | `".js"` | `".ts"`) appends a file extension to | ||
| generated relative imports, for projects using `moduleResolution: node16`/`nodenext` | ||
| (`.js`) or `allowImportingTsExtensions` (`.ts`). | ||
|
|
||
| `typeOnlyImports` emits `import type` for type-only imports (and inline `type` on | ||
| mixed imports such as the http-client import, where `HttpClient` stays a value | ||
| import) for projects using `verbatimModuleSyntax` / `isolatedModules`. `ContentType` | ||
| is only marked `type` for `enumStyle: "union"`, where it is a pure type. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,3 +2,5 @@ | |
| /docs/ | ||
| /node_modules/ | ||
| /tmp-issue-463-run/ | ||
| /.idea/ | ||
| /.serena/ | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,173 @@ | ||
| import * as fs from "node:fs/promises"; | ||
| import * as os from "node:os"; | ||
| import * as path from "node:path"; | ||
| import { afterAll, beforeAll, describe, expect, test } from "vitest"; | ||
| import { generateApi } from "../../../src/index.js"; | ||
|
|
||
| describe("import-file-extension", async () => { | ||
| let tmpdir = ""; | ||
|
|
||
| beforeAll(async () => { | ||
| tmpdir = await fs.mkdtemp(path.join(os.tmpdir(), "swagger-typescript-api")); | ||
| }); | ||
|
|
||
| afterAll(async () => { | ||
| await fs.rm(tmpdir, { recursive: true }); | ||
| }); | ||
|
|
||
| const generate = async ( | ||
| fileName: string, | ||
| options: Partial<Parameters<typeof generateApi>[0]>, | ||
| ) => { | ||
| await generateApi({ | ||
| fileName, | ||
| input: path.resolve(import.meta.dirname, "schema.json"), | ||
| output: tmpdir, | ||
| silent: true, | ||
| modular: true, | ||
|
cubic-dev-ai[bot] marked this conversation as resolved.
|
||
| cleanOutput: false, | ||
| ...options, | ||
| }); | ||
| return fs.readFile(path.join(tmpdir, "Api.ts"), { encoding: "utf8" }); | ||
| }; | ||
|
|
||
| const generateRouteTypes = async ( | ||
| fileName: string, | ||
| options: Partial<Parameters<typeof generateApi>[0]>, | ||
| ) => { | ||
| await generateApi({ | ||
| fileName, | ||
| input: path.resolve(import.meta.dirname, "schema.json"), | ||
| output: tmpdir, | ||
| silent: true, | ||
| modular: true, | ||
| cleanOutput: false, | ||
| generateRouteTypes: true, | ||
| generateClient: false, | ||
| ...options, | ||
| }); | ||
| return fs.readFile(path.join(tmpdir, "ApiRoute.ts"), { encoding: "utf8" }); | ||
| }; | ||
|
|
||
| test('appends ".js" to relative imports', async () => { | ||
| const api = await generate("js-ext", { importFileExtension: ".js" }); | ||
|
|
||
| expect(api).toContain('from "./data-contracts.js"'); | ||
| expect(api).toContain('from "./http-client.js"'); | ||
| }); | ||
|
|
||
| test('appends ".ts" to relative imports', async () => { | ||
| const api = await generate("ts-ext", { importFileExtension: ".ts" }); | ||
|
|
||
| expect(api).toContain('from "./data-contracts.ts"'); | ||
| expect(api).toContain('from "./http-client.ts"'); | ||
| }); | ||
|
|
||
| test("defaults to no extension when option is unset", async () => { | ||
| const api = await generate("no-ext", {}); | ||
|
|
||
| expect(api).toContain('from "./data-contracts"'); | ||
| expect(api).toContain('from "./http-client"'); | ||
| expect(api).not.toContain("data-contracts.js"); | ||
| expect(api).not.toContain("data-contracts.ts"); | ||
| }); | ||
|
|
||
| test("emits whole-block type import for data-contracts when typeOnlyImports", async () => { | ||
| const api = await generate("type-only", { typeOnlyImports: true }); | ||
|
|
||
| expect(api).toMatch(/import type \{[^}]*\} from "\.\/data-contracts"/); | ||
| }); | ||
|
|
||
| test("marks type-only http-client specifiers inline, keeps HttpClient a value import", async () => { | ||
| const api = await generate("type-only-http", { typeOnlyImports: true }); | ||
|
|
||
| const httpImport = api | ||
| .split("\n") | ||
| .find((line) => line.includes('from "./http-client"')); | ||
|
|
||
| expect(httpImport).toBeDefined(); | ||
| expect(httpImport).toContain("type RequestParams"); | ||
| // HttpClient is a runtime class (extended/instantiated) - never type-only | ||
| expect(httpImport).not.toContain("type HttpClient"); | ||
| expect(httpImport).not.toMatch(/import type \{/); | ||
| }); | ||
|
|
||
| test("does not mark ContentType as type for runtime enum styles", async () => { | ||
| const api = await generate("type-only-enum", { | ||
| typeOnlyImports: true, | ||
| enumStyle: "enum", | ||
| }); | ||
|
|
||
| expect(api).not.toContain("type ContentType"); | ||
| }); | ||
|
|
||
| test("never imports ContentType as a runtime value for union enum style", async () => { | ||
| const api = await generate("type-only-union", { | ||
| typeOnlyImports: true, | ||
| enumStyle: "union", | ||
| }); | ||
|
|
||
| const httpImport = api | ||
| .split("\n") | ||
| .find((line) => line.includes('from "./http-client"')); | ||
|
|
||
| expect(httpImport).toBeDefined(); | ||
| // In union mode ContentType is a pure type: procedure calls use string | ||
| // literals instead, so it is either marked `type` or dropped as unused - | ||
| // it must never appear as a bare runtime import. | ||
| expect(httpImport).not.toMatch(/(?<!type )\bContentType\b/); | ||
| // The mixed http-client import still marks the type-only specifiers inline. | ||
| expect(httpImport).toContain("type RequestParams"); | ||
| }); | ||
|
|
||
| test("combines extension and type-only imports", async () => { | ||
| const api = await generate("combined", { | ||
| importFileExtension: ".js", | ||
| typeOnlyImports: true, | ||
| }); | ||
|
|
||
| expect(api).toMatch(/import type \{[^}]*\} from "\.\/data-contracts\.js"/); | ||
| expect(api).toContain('from "./http-client.js"'); | ||
| }); | ||
|
|
||
| test("rejects an unsupported importFileExtension value", async () => { | ||
| await expect( | ||
| generateApi({ | ||
| fileName: "invalid-ext", | ||
| input: path.resolve(import.meta.dirname, "schema.json"), | ||
| output: tmpdir, | ||
| silent: true, | ||
| cleanOutput: false, | ||
| importFileExtension: ".mjs" as ".js", | ||
| }), | ||
| ).rejects.toThrow(/importFileExtension/); | ||
| }); | ||
|
|
||
| test("treats an explicit undefined importFileExtension as no extension", async () => { | ||
| const api = await generate("undef-ext", { | ||
| importFileExtension: undefined, | ||
| }); | ||
|
|
||
| expect(api).toContain('from "./data-contracts"'); | ||
| expect(api).not.toContain("data-contracts.js"); | ||
| }); | ||
|
|
||
| test("appends extension to route-type imports", async () => { | ||
| const routeTypes = await generateRouteTypes("route-types-ext", { | ||
| importFileExtension: ".js", | ||
| }); | ||
|
|
||
| expect(routeTypes).toContain('from "./data-contracts.js"'); | ||
| }); | ||
|
|
||
| test("emits type-only route-type imports when typeOnlyImports", async () => { | ||
| const routeTypes = await generateRouteTypes("route-types-type-only", { | ||
| importFileExtension: ".js", | ||
| typeOnlyImports: true, | ||
| }); | ||
|
|
||
| expect(routeTypes).toMatch( | ||
| /import type \{[^}]*\} from "\.\/data-contracts\.js"/, | ||
| ); | ||
| }); | ||
| }); | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
P2: When a CLI user passes a value other than
"",.js, or.ts, this flag accepts it and the cast does not validate it at runtime. Validate and reject unsupported extensions before passing the value togenerateApi, otherwise generation can produce imports that do not point to generated files.Prompt for AI agents