diff --git a/eslint.config.js b/eslint.config.js index c0090f571..4197ace1c 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -72,6 +72,10 @@ const performanceConcerns = [ selector: "ImportDeclaration[source.value=/package.json$/]", // #2974 message: "it can not be tree shaken, use tsdown and process.env instead", }, + { + selector: "CallExpression[callee.property.name=/^(shift|unshift)$/]", // #3343 + message: "shifting is 2-20x slower than index-based iteration", + }, ]; const tsFactoryConcerns = [ diff --git a/express-zod-api/bench/can-merge.bench.ts b/express-zod-api/bench/can-merge.bench.ts deleted file mode 100644 index 0a81069b0..000000000 --- a/express-zod-api/bench/can-merge.bench.ts +++ /dev/null @@ -1,64 +0,0 @@ -import { bench, describe } from "vitest"; -import * as R from "ramda"; - -const mergeableKeys = new Set([ - "type", - "properties", - "required", - "examples", - "description", - "additionalProperties", -]); - -const current = (subject: Record) => - R.pipe( - Object.keys, - R.without([ - "type", - "properties", - "required", - "examples", - "description", - "additionalProperties", - ] as string[]), - R.isEmpty, - )(subject); - -const featured = (subject: Record): boolean => { - for (const key of Object.keys(subject)) - if (!mergeableKeys.has(key)) return false; - return true; -}; - -describe.each([ - {}, - { type: "object" }, - { type: "object", properties: {} }, - { type: "object", properties: {}, required: [] }, - { type: "object", properties: {}, required: [], examples: [] }, - { - type: "object", - properties: {}, - required: [], - examples: [], - description: "test", - }, - { - type: "object", - properties: {}, - required: [], - examples: [], - description: "test", - additionalProperties: false, - }, - { type: "object", title: "test" }, - { type: "object", format: "date-time", title: "test" }, -])("Experiment for canMerge %#", (subject) => { - bench(`current`, () => { - current(subject); - }); - - bench(`featured`, () => { - featured(subject); - }); -}); diff --git a/express-zod-api/bench/queue-ops.bench.ts b/express-zod-api/bench/queue-ops.bench.ts new file mode 100644 index 000000000..80bf84c46 --- /dev/null +++ b/express-zod-api/bench/queue-ops.bench.ts @@ -0,0 +1,42 @@ +import { bench, describe } from "vitest"; + +const smallQueue = Array.from({ length: 5 }, (_, i) => ({ + id: i, + data: `item-${i}`, +})); +const mediumQueue = Array.from({ length: 20 }, (_, i) => ({ + id: i, + data: `item-${i}`, +})); +const largeQueue = Array.from({ length: 100 }, (_, i) => ({ + id: i, + data: `item-${i}`, +})); + +const queues = [ + [smallQueue, "small (5)"], + [mediumQueue, "medium (20)"], + [largeQueue, "large (100)"], +] as const; + +describe.each(queues)("$1", (queue) => { + bench("shift() approach", () => { + const q = [...queue]; + while (q.length) q.shift(); + }); + + bench("index approach", () => { + const q = [...queue]; + for (let idx = 0; idx < q.length; idx++) q[idx]; // eslint-disable-line @typescript-eslint/no-unused-expressions + }); + + bench("pop() reverse", () => { + const q = [...queue].reverse(); + while (q.length) q.pop(); + }); + + bench("forEach", () => { + const q = [...queue]; + q.forEach(() => {}); + }); +}); diff --git a/express-zod-api/src/deep-checks.ts b/express-zod-api/src/deep-checks.ts index 0b167ad15..7a4eb2586 100644 --- a/express-zod-api/src/deep-checks.ts +++ b/express-zod-api/src/deep-checks.ts @@ -39,8 +39,8 @@ export const hasCycle = ( ) => { const json = z.toJSONSchema(subject, { io, unrepresentable: "any" }); const stack: unknown[] = [json]; - while (stack.length) { - const entry = stack.shift()!; + for (let idx = 0; idx < stack.length; idx++) { + const entry = stack[idx]; if (R.is(Object, entry)) { if ((entry as z.core.JSONSchema.BaseSchema).$ref === "#") return true; stack.push(...R.values(entry)); diff --git a/express-zod-api/src/diagnostics.ts b/express-zod-api/src/diagnostics.ts index 6e96515b7..c21b602fe 100644 --- a/express-zod-api/src/diagnostics.ts +++ b/express-zod-api/src/diagnostics.ts @@ -29,8 +29,8 @@ export class Diagnostics { const stack: z.core.JSONSchema.BaseSchema[] = [ z.toJSONSchema(endpoint[`${dir}Schema`], { unrepresentable: "any" }), ]; - while (stack.length > 0) { - const entry = stack.shift()!; + for (let idx = 0; idx < stack.length; idx++) { + const entry = stack[idx]; if (entry.type && entry.type !== "object") this.logger.warn(`Endpoint ${dir} schema is not object-based`, ctx); for (const prop of ["allOf", "oneOf", "anyOf"] as const) diff --git a/express-zod-api/src/documentation-helpers.ts b/express-zod-api/src/documentation-helpers.ts index c9a646929..cf9b78c26 100644 --- a/express-zod-api/src/documentation-helpers.ts +++ b/express-zod-api/src/documentation-helpers.ts @@ -371,8 +371,8 @@ const fixReferences = ( ctx: OpenAPIContext, ) => { const stack: unknown[] = [subject, defs]; - while (stack.length) { - const entry = stack.shift()!; + for (let idx = 0; idx < stack.length; idx++) { + const entry = stack[idx]; if (R.is(Object, entry)) { if (isReferenceObject(entry) && !entry.$ref.startsWith("#/components")) { const actualName = entry.$ref.split("/").pop()!; diff --git a/express-zod-api/src/integration.ts b/express-zod-api/src/integration.ts index 1fe253e34..46f50438c 100644 --- a/express-zod-api/src/integration.ts +++ b/express-zod-api/src/integration.ts @@ -153,6 +153,7 @@ export class Integration extends IntegrationBase { config, onEndpoint: hasHeadMethod ? withHead(onEndpoint) : onEndpoint, }); + // eslint-disable-next-line no-restricted-syntax -- accumulated late, acceptable for generator this.#program.unshift(...this.#aliases.values()); this.#program.push( this.makePathType(), diff --git a/express-zod-api/src/json-schema-helpers.ts b/express-zod-api/src/json-schema-helpers.ts index daa103505..3eceb01a1 100644 --- a/express-zod-api/src/json-schema-helpers.ts +++ b/express-zod-api/src/json-schema-helpers.ts @@ -107,8 +107,8 @@ export const flattenIO = ( const stack: Stack = [R.pair(false, jsonSchema)]; // [isOptional, JSON Schema] const flat: FlattenObjectSchema = { type: "object", properties: {} }; const flatRequired: string[] = []; - while (stack.length) { - const [isOptional, entry] = stack.shift()!; + for (let idx = 0; idx < stack.length; idx++) { + const [isOptional, entry] = stack[idx]; if (entry.description) flat.description ??= entry.description; stack.push(...processAllOf(entry, mode, isOptional)); stack.push(...processVariants(entry)); diff --git a/express-zod-api/src/routing-walker.ts b/express-zod-api/src/routing-walker.ts index 9dba9564e..27f1177b8 100644 --- a/express-zod-api/src/routing-walker.ts +++ b/express-zod-api/src/routing-walker.ts @@ -91,8 +91,8 @@ export const walkRouting = ({ }: RoutingWalkerParams) => { const stack = processEntries(config, routing); const visited = new Set(); - while (stack.length) { - const [path, element, explicitMethod] = stack.shift()!; + for (let idx = 0; idx < stack.length; idx++) { + const [path, element, explicitMethod] = stack[idx]; if (element instanceof AbstractEndpoint) { if (explicitMethod) { checkDuplicate(explicitMethod, path, visited); @@ -110,7 +110,7 @@ export const walkRouting = ({ if (element instanceof ServeStatic) { if (onStatic) element.apply(path, onStatic); } else { - stack.unshift(...processEntries(config, element, path)); + stack.splice(idx + 1, 0, ...processEntries(config, element, path)); } } } diff --git a/express-zod-api/src/routing.ts b/express-zod-api/src/routing.ts index c9b5d7cea..7011b9ab9 100644 --- a/express-zod-api/src/routing.ts +++ b/express-zod-api/src/routing.ts @@ -89,15 +89,9 @@ export const initRouting = ({ app, config, getLogger, ...rest }: InitProps) => { /** @link https://github.com/RobinTail/express-zod-api/discussions/2791#discussioncomment-13745912 */ if (accessMethods.includes("get")) accessMethods.push("head"); for (const [method, [matchingParsers, endpoint]] of methods) { - const handlers = matchingParsers - .slice() // must be immutable - .concat(async (request, response) => { - const logger = getLogger(request); - return endpoint.execute({ request, response, logger, config }); - }); + const handlers: RequestHandler[] = []; // issue #2706: CORS must go before parsers: if (config.cors) { - // issue #2706, must go before parsers: - handlers.unshift(async (request, response, next) => { + handlers.push(async (request, response, next) => { const logger = getLogger(request); const defaultHeaders = makeCorsHeaders(accessMethods); const headers = @@ -108,6 +102,10 @@ export const initRouting = ({ app, config, getLogger, ...rest }: InitProps) => { next(); }); } + handlers.push(...matchingParsers, async (request, response) => { + const logger = getLogger(request); + return endpoint.execute({ request, response, logger, config }); + }); app[method](path, ...handlers); } if (config.wrongMethodBehavior === 404) continue; diff --git a/express-zod-api/src/zts.ts b/express-zod-api/src/zts.ts index db4d7e36a..0b82f8dc2 100644 --- a/express-zod-api/src/zts.ts +++ b/express-zod-api/src/zts.ts @@ -40,27 +40,27 @@ const onTemplateLiteral: Producer = ( { _zod: { def } }: z.core.$ZodTemplateLiteral, { next, api }, ) => { - const parts = [...def.parts]; - const shiftText = () => { + const { parts } = def; + let idx = 0; + const readText = () => { let text = ""; - while (parts.length) { - const part = parts.shift(); - if (isSchema(part)) { - parts.unshift(part); - break; - } + while (idx < parts.length) { + const part = parts[idx]; + if (isSchema(part)) break; + idx++; text += part ?? ""; // Handle potential undefined values } return text; }; - const head = api.f.createTemplateHead(shiftText()); + const head = api.f.createTemplateHead(readText()); const spans: ts.TemplateLiteralTypeSpan[] = []; - while (parts.length) { - const schema = next(parts.shift() as z.core.$ZodType); - const text = shiftText(); - const textWrapper = parts.length - ? api.f.createTemplateMiddle - : api.f.createTemplateTail; + while (idx < parts.length) { + const schema = next(parts[idx++] as z.core.$ZodType); + const text = readText(); + const textWrapper = + idx < parts.length + ? api.f.createTemplateMiddle + : api.f.createTemplateTail; spans.push(api.f.createTemplateLiteralTypeSpan(schema, textWrapper(text))); } if (!spans.length) return api.makeLiteralType(head.text); diff --git a/express-zod-api/tests/__snapshots__/routing-walker.spec.ts.snap b/express-zod-api/tests/__snapshots__/routing-walker.spec.ts.snap new file mode 100644 index 000000000..260b71ae6 --- /dev/null +++ b/express-zod-api/tests/__snapshots__/routing-walker.spec.ts.snap @@ -0,0 +1,36 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`walkRouting() > should process endpoints in depth-first order 0 1`] = ` +[ + [ + "get", + "/v1/user/retrieve", + Endpoint {}, + ], + [ + "get", + "/v1/user/create", + Endpoint {}, + ], + [ + "get", + "/v1/record", + Endpoint {}, + ], +] +`; + +exports[`walkRouting() > should process endpoints in depth-first order 1 1`] = ` +[ + [ + "get", + "/a/b/c", + Endpoint {}, + ], + [ + "get", + "/a/d", + Endpoint {}, + ], +] +`; diff --git a/express-zod-api/tests/routing-walker.spec.ts b/express-zod-api/tests/routing-walker.spec.ts new file mode 100644 index 000000000..c10479ddc --- /dev/null +++ b/express-zod-api/tests/routing-walker.spec.ts @@ -0,0 +1,27 @@ +import { defaultEndpointsFactory, Routing } from "../src"; +import { walkRouting } from "../src/routing-walker"; + +describe("walkRouting()", () => { + const endpoint = defaultEndpointsFactory.buildVoid({ + handler: vi.fn(), + }); + + const onEndpoint = vi.fn(); + + afterEach(() => { + onEndpoint.mockClear(); + }); + + test.each([ + { + v1: { + user: { retrieve: endpoint, create: endpoint }, + record: endpoint, + }, + }, + { a: { b: { c: endpoint }, d: endpoint } }, + ])("should process endpoints in depth-first order %#", (routing) => { + walkRouting({ routing, config: { cors: false }, onEndpoint }); + expect(onEndpoint.mock.calls).toMatchSnapshot(); + }); +});