Skip to content

Fix mutating $refs in Node.js API #1203

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 1 commit into from
Jul 6, 2023
Merged

Fix mutating $refs in Node.js API #1203

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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/fresh-dancers-joke.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"openapi-typescript": patch
---

Fix mutating $refs in Node.js API
2 changes: 1 addition & 1 deletion packages/openapi-typescript/src/load.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ export default async function load(schema: URL | Subschema | Readable, options:
else if (typeof schema === "object") {
options.schemas[schemaID] = {
hint: "OpenAPI3",
schema: schema as any,
schema: JSON.parse(JSON.stringify(schema)), // create deep clone of inline schema (don’t mutate)
Copy link
Contributor Author

@drwpow drwpow Jul 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

So funny enough, after some lazy performance testing in V8, JSON.parse(JSON.stringify(…)) was faster than structuredClone(). Which I guess makes sense with JSON-serializable data that we’re working with. I imagine it‘s also because structuredClone() hasn’t had as much time to be optimized, comparatively. But I’d hope long-term the latter would be faster.

This is probably not that much of a hack as I originally thought it was. And this only runs in the Node.js API (only when needed)

};
}
// 1d. failsafe
Expand Down
42 changes: 28 additions & 14 deletions packages/openapi-typescript/test/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,10 +189,7 @@ export type operations = Record<string, never>;
"/post/{id}": {
get: {
operationId: "getPost",
parameters: [
{ name: "format", in: "query", schema: { type: "string" } },
{ $ref: "#/components/parameters/post_id" },
],
parameters: [{ name: "format", in: "query", schema: { type: "string" } }, { $ref: "#/components/parameters/post_id" }],
responses: {
200: {
description: "OK",
Expand Down Expand Up @@ -447,11 +444,7 @@ export type operations = Record<string, never>;
components: {
schemas: {
Pet: {
oneOf: [
{ $ref: "#/components/schemas/Cat" },
{ $ref: "#/components/schemas/Dog" },
{ $ref: "#/components/schemas/Lizard" },
],
oneOf: [{ $ref: "#/components/schemas/Cat" }, { $ref: "#/components/schemas/Dog" }, { $ref: "#/components/schemas/Lizard" }],
discriminator: {
propertyName: "petType",
mapping: {
Expand Down Expand Up @@ -539,10 +532,7 @@ export type operations = Record<string, never>;
},
},
AllOf: {
allOf: [
{ $ref: "#/components/schemas/Entity/properties/foo" },
{ $ref: "#/components/schemas/Thingy/properties/bar" },
],
allOf: [{ $ref: "#/components/schemas/Entity/properties/foo" }, { $ref: "#/components/schemas/Thingy/properties/bar" }],
},
},
},
Expand Down Expand Up @@ -730,7 +720,7 @@ export type operations = Record<string, never>;
},
put: {
parameters: [{ name: "user_id", in: "path" }],
}
},
},
},
};
Expand Down Expand Up @@ -982,6 +972,30 @@ export type operations = Record<string, never>;
});
});

it("does not mutate original reference", async () => {
const schema: OpenAPI3 = {
openapi: "3.1",
info: { title: "test", version: "1.0" },
components: {},
paths: {
"/": {
get: {
responses: {
200: {
description: "ok",
$ref: "#/components/schemas/OKResponse",
},
},
},
},
},
};
const before = JSON.stringify(schema);
await openapiTS(schema);
const after = JSON.stringify(schema);
expect(before).toBe(after);
});

// note: this tests the Node API; the snapshots in cli.test.ts test the CLI
describe("snapshots", () => {
const EXAMPLES_DIR = new URL("../examples/", import.meta.url);
Expand Down