-
Notifications
You must be signed in to change notification settings - Fork 36
fix(ts-interface-generator): pass process.argv to yargs so CLI flags work #573
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
Merged
+309
−1
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
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
307 changes: 307 additions & 0 deletions
307
packages/ts-interface-generator/src/test/jsdocPreference.test.ts
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,307 @@ | ||
| import fs from "fs"; | ||
| import path from "path"; | ||
| import ts from "typescript"; | ||
| import log from "loglevel"; | ||
| import { execSync, execFileSync } from "child_process"; | ||
| import { generateInterfaces } from "../interfaceGenerationHelper"; | ||
| import { | ||
| getAllKnownGlobals, | ||
| GlobalToModuleMapping, | ||
| } from "../typeScriptEnvironment"; | ||
| import { getProgramInfo } from "../generateTSInterfacesAPI"; | ||
| import Preferences from "../preferences"; | ||
|
|
||
| jest.setTimeout(30000); | ||
|
|
||
| const testCasesDir = path.resolve(__dirname, "testcases"); | ||
|
|
||
| const standardTsConfig: ts.CompilerOptions = { | ||
| target: ts.ScriptTarget.ES2022, | ||
| module: ts.ModuleKind.CommonJS, | ||
| strict: true, | ||
| moduleResolution: ts.ModuleResolutionKind.Node16, | ||
| esModuleInterop: true, | ||
| skipLibCheck: true, | ||
| forceConsistentCasingInFileNames: true, | ||
| types: ["openui5"], | ||
| }; | ||
|
|
||
| function generateForTestCase(testCaseDir: string): Promise<string> { | ||
| const config = { ...standardTsConfig, baseUrl: testCaseDir }; | ||
| const tsFiles = fs | ||
| .readdirSync(testCaseDir) | ||
| .filter((file) => file.endsWith(".ts") && !file.endsWith(".d.ts")) | ||
| .map((file) => path.join(testCaseDir, file)); | ||
|
|
||
| const program = ts.createProgram(tsFiles, config); | ||
| const typeChecker = program.getTypeChecker(); | ||
| const programInfo = getProgramInfo(program, typeChecker); | ||
| const allKnownGlobals: GlobalToModuleMapping = | ||
| getAllKnownGlobals(typeChecker); | ||
|
|
||
| const sourceFiles = program.getSourceFiles().filter((sourceFile) => { | ||
| return ( | ||
| !sourceFile.isDeclarationFile && | ||
| path.basename(sourceFile.fileName) !== "library.ts" | ||
| ); | ||
| }); | ||
|
|
||
| return new Promise((resolve) => { | ||
| const resultProcessor = ( | ||
| _sourceFileName: string, | ||
| _className: string, | ||
| interfaceText: string, | ||
| ) => { | ||
| resolve(interfaceText); | ||
| }; | ||
|
|
||
| generateInterfaces( | ||
| sourceFiles[0], | ||
| typeChecker, | ||
| Object.assign({}, programInfo.allKnownLocalExports, allKnownGlobals), | ||
| resultProcessor, | ||
| ); | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Tests for issue #542: --jsdoc CLI parameter is not respected. | ||
| * | ||
| * Root cause: In commit bf53c43 (yargs v17→v18 upgrade), the CLI entry point was | ||
| * rewritten from the singleton pattern (yargs.option({...}).argv — which reads | ||
| * process.argv) to the factory pattern (yargs().option({...}).argv). The factory | ||
| * pattern creates a detached instance that does NOT read process.argv in either | ||
| * v17 or v18. The fix is to pass hideBin(process.argv) explicitly: | ||
| * yargs(hideBin(process.argv)).option({...}).argv | ||
| */ | ||
|
|
||
| describe("JSDoc CLI argument parsing (root cause of issue #542)", () => { | ||
| test("yargs() without arguments ignores process.argv in v18 — returns default 'verbose'", () => { | ||
| const script = path.join(__dirname, "_yargs_test_no_hideBin.mjs"); | ||
| fs.writeFileSync( | ||
| script, | ||
| `import yargs from 'yargs'; | ||
| const argv = await yargs().option({ jsdoc: { choices: ['none','minimal','verbose'], default: 'verbose' } }).argv; | ||
| process.stdout.write(argv.jsdoc);`, | ||
| ); | ||
| try { | ||
| const result = execSync(`node ${script} --jsdoc minimal`, { | ||
| encoding: "utf-8", | ||
| }); | ||
|
akudev marked this conversation as resolved.
|
||
| expect(result).toBe("verbose"); | ||
| } finally { | ||
| fs.unlinkSync(script); | ||
| } | ||
| }); | ||
|
|
||
| test("yargs(hideBin(process.argv)) correctly parses --jsdoc minimal", () => { | ||
| const script = path.join(__dirname, "_yargs_test_with_hideBin.mjs"); | ||
| fs.writeFileSync( | ||
| script, | ||
| `import yargs from 'yargs'; | ||
| import { hideBin } from 'yargs/helpers'; | ||
| const argv = await yargs(hideBin(process.argv)).option({ jsdoc: { choices: ['none','minimal','verbose'], default: 'verbose' } }).argv; | ||
| process.stdout.write(argv.jsdoc);`, | ||
| ); | ||
| try { | ||
| const result = execSync(`node ${script} --jsdoc minimal`, { | ||
| encoding: "utf-8", | ||
| }); | ||
|
akudev marked this conversation as resolved.
|
||
| expect(result).toBe("minimal"); | ||
| } finally { | ||
| fs.unlinkSync(script); | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| describe("CLI end-to-end (issue #542)", () => { | ||
| const cliEntryPoint = path.resolve( | ||
| __dirname, | ||
| "../../dist/generateTSInterfaces.js", | ||
| ); | ||
| const testCaseDir = path.resolve( | ||
| __dirname, | ||
| "testcases/tsconfig-path-relative", | ||
| ); | ||
| const genFile = path.join(testCaseDir, "MyControl.gen.d.ts"); | ||
|
|
||
| beforeAll(() => { | ||
| if (!fs.existsSync(cliEntryPoint)) { | ||
| execSync("npx tsc", { cwd: path.resolve(__dirname, "../..") }); | ||
| } | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| // Restore original gen files | ||
| execSync("git checkout -- .", { cwd: testCaseDir }); | ||
| }); | ||
|
|
||
| test("--jsdoc minimal produces output without boilerplate JSDoc", () => { | ||
| fs.unlinkSync(genFile); | ||
| execFileSync("node", [cliEntryPoint, "--jsdoc", "minimal"], { | ||
| cwd: testCaseDir, | ||
| }); | ||
| const output = fs.readFileSync(genFile, "utf-8"); | ||
|
|
||
| expect(output).toContain("getMyJSEnumVal(): MyJSEnum;"); | ||
| expect(output).not.toContain("@returns"); | ||
| expect(output).not.toContain("@param"); | ||
| expect(output).not.toContain("Gets current value of property"); | ||
| }); | ||
|
|
||
| test("--jsdoc none produces output without any method-level JSDoc", () => { | ||
| fs.unlinkSync(genFile); | ||
| execFileSync("node", [cliEntryPoint, "--jsdoc", "none"], { | ||
| cwd: testCaseDir, | ||
| }); | ||
| const output = fs.readFileSync(genFile, "utf-8"); | ||
|
|
||
| expect(output).toContain("getMyJSEnumVal(): MyJSEnum;"); | ||
| expect(output).not.toContain("@returns"); | ||
| expect(output).not.toContain("@param"); | ||
| }); | ||
|
|
||
| test("--jsdoc verbose produces output with full boilerplate JSDoc", () => { | ||
| fs.unlinkSync(genFile); | ||
| execFileSync("node", [cliEntryPoint, "--jsdoc", "verbose"], { | ||
| cwd: testCaseDir, | ||
| }); | ||
| const output = fs.readFileSync(genFile, "utf-8"); | ||
|
|
||
| expect(output).toContain("@returns"); | ||
| expect(output).toContain("@param"); | ||
| expect(output).toContain('Gets current value of property "myJSEnumVal"'); | ||
| }); | ||
| }); | ||
|
|
||
| describe("JSDoc preference modes", () => { | ||
| beforeAll(() => { | ||
| jest.spyOn(log, "warn").mockImplementation(() => {}); | ||
| }); | ||
|
|
||
| afterAll(() => { | ||
| jest.restoreAllMocks(); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| Preferences.set({ jsdoc: "verbose" }); | ||
| }); | ||
|
|
||
| const xlControlDir = path.join(testCasesDir, "xl-control-with-all-features"); | ||
| const simpleControlDir = path.join(testCasesDir, "simple-control"); | ||
|
|
||
| describe("simple-control (no source JSDoc on properties)", () => { | ||
| test("verbose mode includes boilerplate JSDoc", async () => { | ||
| Preferences.set({ jsdoc: "verbose" }); | ||
| const result = await generateForTestCase(simpleControlDir); | ||
|
|
||
| expect(result).toContain('Gets current value of property "text"'); | ||
| expect(result).toContain('@returns Value of property "text"'); | ||
| expect(result).toContain('Sets a new value for property "text"'); | ||
| expect(result).toContain('@param text New value for property "text"'); | ||
| expect(result).toContain( | ||
| '@returns Reference to "this" in order to allow method chaining', | ||
| ); | ||
| }); | ||
|
|
||
| test("minimal mode omits boilerplate JSDoc for properties without source doc", async () => { | ||
| Preferences.set({ jsdoc: "minimal" }); | ||
| const result = await generateForTestCase(simpleControlDir); | ||
|
|
||
| expect(result).not.toContain('Gets current value of property "text"'); | ||
| expect(result).not.toContain('@returns Value of property "text"'); | ||
| expect(result).not.toContain('@param text New value for property "text"'); | ||
| expect(result).not.toContain( | ||
| '@returns Reference to "this" in order to allow method chaining', | ||
| ); | ||
| }); | ||
|
|
||
| test("none mode produces no method-level JSDoc comments", async () => { | ||
| Preferences.set({ jsdoc: "none" }); | ||
| const result = await generateForTestCase(simpleControlDir); | ||
|
|
||
| expect(result).not.toContain('Gets current value of property "text"'); | ||
| expect(result).not.toContain('@returns Value of property "text"'); | ||
| expect(result).not.toContain("@param text"); | ||
| // The settings interface description comment is not gated by jsdoc preference (known behavior) | ||
| expect(result).toContain( | ||
| "Interface defining the settings object used in constructor calls", | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| describe("xl-control-with-all-features (has source JSDoc, @since, @experimental)", () => { | ||
| test("verbose mode includes both boilerplate and source JSDoc", async () => { | ||
| Preferences.set({ jsdoc: "verbose" }); | ||
| const result = await generateForTestCase(xlControlDir); | ||
|
|
||
| // boilerplate | ||
| expect(result).toContain('Gets current value of property "subtext"'); | ||
| expect(result).toContain('@returns Value of property "subtext"'); | ||
| // source doc | ||
| expect(result).toContain("The text that appears below the main text."); | ||
| expect(result).toContain("@since 1.0"); | ||
| expect(result).toContain("@experimental"); | ||
| }); | ||
|
|
||
| test("minimal mode keeps source JSDoc but removes boilerplate", async () => { | ||
| Preferences.set({ jsdoc: "minimal" }); | ||
| const result = await generateForTestCase(xlControlDir); | ||
|
|
||
| // source doc and tags should still be present | ||
| expect(result).toContain("The text that appears below the main text."); | ||
| expect(result).toContain("@since 1.0"); | ||
| expect(result).toContain("@experimental"); | ||
| expect(result).toContain("Determines the text color of the"); | ||
|
|
||
| // boilerplate should be absent | ||
| expect(result).not.toContain('Gets current value of property "subtext"'); | ||
| expect(result).not.toContain('@returns Value of property "subtext"'); | ||
| expect(result).not.toContain('Sets a new value for property "subtext"'); | ||
| expect(result).not.toContain( | ||
| '@param subtext New value for property "subtext"', | ||
| ); | ||
| expect(result).not.toContain('Attaches event handler "fn" to the'); | ||
| expect(result).not.toContain('Detaches event handler "fn" from the'); | ||
| expect(result).not.toContain( | ||
| 'Fires event "singlePress" to attached listeners.', | ||
| ); | ||
| }); | ||
|
|
||
| test("none mode produces no method-level JSDoc comments", async () => { | ||
| Preferences.set({ jsdoc: "none" }); | ||
| const result = await generateForTestCase(xlControlDir); | ||
|
|
||
| // Method-level JSDoc should be absent | ||
| expect(result).not.toContain("@returns Value of property"); | ||
| expect(result).not.toContain("@param subtext"); | ||
| expect(result).not.toContain("Gets current value of property"); | ||
| expect(result).not.toContain("Attaches event handler"); | ||
| // Source-level tags should also be absent | ||
| expect(result).not.toContain("@since"); | ||
| expect(result).not.toContain("@experimental"); | ||
| }); | ||
|
|
||
| test("verbose and minimal produce different output", async () => { | ||
| Preferences.set({ jsdoc: "verbose" }); | ||
| const verbose = await generateForTestCase(xlControlDir); | ||
|
|
||
| Preferences.set({ jsdoc: "minimal" }); | ||
| const minimal = await generateForTestCase(xlControlDir); | ||
|
|
||
| expect(verbose).not.toEqual(minimal); | ||
| expect(verbose.length).toBeGreaterThan(minimal.length); | ||
| }); | ||
|
|
||
| test("minimal and none produce different output", async () => { | ||
| Preferences.set({ jsdoc: "minimal" }); | ||
| const minimal = await generateForTestCase(xlControlDir); | ||
|
|
||
| Preferences.set({ jsdoc: "none" }); | ||
| const none = await generateForTestCase(xlControlDir); | ||
|
|
||
| expect(minimal).not.toEqual(none); | ||
| expect(minimal.length).toBeGreaterThan(none.length); | ||
| }); | ||
| }); | ||
| }); | ||
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.
Uh oh!
There was an error while loading. Please reload this page.