Skip to content

Do not append trailing spaces to JSDoc tags #1231

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

Merged
merged 4 commits into from
Jul 21, 2023
Merged

Do not append trailing spaces to JSDoc tags #1231

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b74f986
Fix pnpm format script
tkrotoff Jul 20, 2023
3a9b1bb
Do not append trailing spaces to JSDoc tags
tkrotoff Jul 20, 2023
9274c5e
Further remove trailing spaces
tkrotoff Jul 20, 2023
b3c3508
Update examples
tkrotoff Jul 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/funny-lies-tease.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"openapi-typescript": patch
---

Do not append trailing spaces to JSDoc tags
6 changes: 3 additions & 3 deletions packages/openapi-typescript/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@
"build": "del dist && tsc -p tsconfig.build.json",
"dev": "tsc -p tsconfig.build.json --watch",
"download:schemas": "vite-node ./scripts/download-schemas.ts",
"format": "pnpm run prettier -w .",
"format": "prettier --write 'src/**/*'",
"lint": "run-p -s lint:*",
"lint:js": "eslint \"src/**/*.{js,ts}\"",
"lint:prettier": "prettier --check \"src/**/*\"",
"lint:js": "eslint 'src/**/*.{js,ts}'",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"lint:js": "eslint 'src/**/*.{js,ts}'",
"lint:js": "eslint \"src/**/*.{js,ts}\"",

We actually want to keep the escaped double quotes. Though *nix and mac users can run this just fine, IIRC Windows contributors run into problems with single quotes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is such a minor thing, I’ll revert this after merge, but won’t touch anything else. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for that I always work on a UNIX OS and though the scripts without escaped quotes look nicer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh no worries at all. I did the same thing until I had Windows users complain 😛 (and I agree with you)

"lint:prettier": "prettier --check 'src/**/*'",
"prepare": "pnpm run build",
"test": "run-p -s test:*",
"test:js": "vitest run",
Expand Down
12 changes: 6 additions & 6 deletions packages/openapi-typescript/src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,13 @@ export function getSchemaObjectComment(v: CommentObject, indentLv?: number): str
const output: string[] = [];

// * Not JSDOC tags: [title, format]
if (v.title) output.push(`${v.title} `);
if (v.summary) output.push(`${v.summary} `);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think some of these string escapes mostly came from fields that could contain non-strings, but I think that was really only example. I don’t see the harm in removing the string escapes.

if (v.format) output.push(`Format: ${v.format} `);
if (v.title) output.push(v.title);
if (v.summary) output.push(v.summary);
if (v.format) output.push(`Format: ${v.format}`);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment function already does the job of adding a trailing space in case of a single-line comment


// * JSDOC tags without value
// 'Deprecated' without value
if (v.deprecated) output.push(`@deprecated `);
if (v.deprecated) output.push("@deprecated");

// * JSDOC tags with value
const supportedJsDocTags: (keyof CommentObject)[] = ["description", "default", "example"];
Expand All @@ -74,11 +74,11 @@ export function getSchemaObjectComment(v: CommentObject, indentLv?: number): str
continue;
}
const serialized = typeof v[field] === "object" ? JSON.stringify(v[field], null, 2) : v[field];
output.push(`@${field} ${serialized} `);
output.push(`@${field} ${serialized}`);
}

// * JSDOC 'Constant' without value
if ("const" in v) output.push(`@constant `);
if ("const" in v) output.push("@constant");

// * JSDOC 'Enum' with type
if (v.enum) {
Expand Down
56 changes: 55 additions & 1 deletion packages/openapi-typescript/test/utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { bench } from "vitest";
import { escObjKey, parseRef, tsIntersectionOf, tsUnionOf } from "../src/utils.js";
import { comment, escObjKey, getSchemaObjectComment, parseRef, tsIntersectionOf, tsUnionOf } from "../src/utils.js";

describe("utils", () => {
describe("tsUnionOf", () => {
Expand Down Expand Up @@ -112,4 +112,58 @@ describe("utils", () => {
expect(escObjKey("_ref_")).toStrictEqual("_ref_");
});
});

describe("comment", () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great tests 👏

it("basic", () => {
expect(comment("A comment")).toStrictEqual("/** A comment */");
expect(comment("A multi-line \n comment")).toStrictEqual(
// prettier-ignore
"/**\n" +
" * A multi-line \n" +
" * comment\n"+
" */"
);
});
});

describe("getSchemaObjectComment", () => {
it("object with 1 property", () => {
expect(
getSchemaObjectComment({
title: "A title",
})
).toStrictEqual("/** A title */");
});

it("object with 2 properties", () => {
expect(
getSchemaObjectComment({
title: "A title",
description: "A description",
})
).toStrictEqual(
// prettier-ignore
"/**\n" +
" * A title\n" +
" * @description A description\n"+
" */"
);
});

it("object with a multi-line property", () => {
expect(
getSchemaObjectComment({
title: "A title",
description: "A multi-line \n description",
})
).toStrictEqual(
// prettier-ignore
"/**\n" +
" * A title\n" +
" * @description A multi-line \n" +
" * description\n" +
" */"
);
});
});
});