openapi-zod-client icon indicating copy to clipboard operation
openapi-zod-client copied to clipboard

Regular expression string pattern in OAS yield uncompilable typescript due to unnecessary escaping

Open mjperrone opened this issue 2 years ago • 12 comments

Minimal spec with a regex that has an escape character:

openapi: 3.0.0
info:
  version: 1.0.0
  title: API for Regex Test
paths:
  /test:
    get:
      responses:
        '200':
          description: A test endpoint
          content:
            application/json:
              schema:
                type: object
                properties:
                  regexmatch:
                    type: string
                    pattern: .+\/.+

Output:

import { makeApi, Zodios, type ZodiosOptions } from "@zodios/core";
import { z } from "zod";






const endpoints = makeApi([
	{
		method: "get",
		path: "/test",
		alias: "getTest",
		requestFormat: "json",
		response: z.object({ regexmatch: z.string().regex(/.+\\/.+/) }).partial().passthrough(),
	},
]);

export const api = new Zodios(endpoints);

export function createApiClient(baseUrl: string, options?: ZodiosOptions) {
    return new Zodios(baseUrl, endpoints, options);
}

This does not compile. Compile errors:

tsc
src/minimalout.ts:15:60 - error TS1003: Identifier expected.

15   response: z.object({ regexmatch: z.string().regex(/.+\\/.+/) }).partial().passthrough(),
                                                              ~

src/minimalout.ts:15:62 - error TS1161: Unterminated regular expression literal.

15   response: z.object({ regexmatch: z.string().regex(/.+\\/.+/) }).partial().passthrough(),
                                                                

src/minimalout.ts:16:2 - error TS1005: ',' expected.

16  },
    ~

src/minimalout.ts:17:1 - error TS1135: Argument expression expected.

17 ]);
   ~


Found 4 errors in the same file, starting at: src/minimalout.ts:15

error Command failed with exit code 2.

Probably the right output for the regexmatch would be:

z.string().regex(/.+\/.+/)

Probably this is implemented here or maybe here.

mjperrone avatar Aug 05 '23 03:08 mjperrone

Here is a pull request containing a failing test showing this behavior: https://github.com/astahmer/openapi-zod-client/pull/197

mjperrone avatar Aug 07 '23 17:08 mjperrone

good catch ! if you do want to fix that, please add a changeset 🙏

astahmer avatar Aug 07 '23 19:08 astahmer

I haven't used changeset before, but I attempted to do that.

mjperrone avatar Aug 08 '23 18:08 mjperrone

I see that you merged https://github.com/astahmer/openapi-zod-client/pull/197. I'm interested in trying it out, but it looks like there was not a new patch version published. Was that intentional?

mjperrone avatar Aug 09 '23 18:08 mjperrone

yes, a release only happens when merging the PR that the changeset CLI creates

this one for example https://github.com/astahmer/openapi-zod-client/pull/200

you can still try the PR by going on the branch, i'll wait for your confirmation before merging the release PR

astahmer avatar Aug 09 '23 18:08 astahmer

@astahmer it looks like you merged the PRs without my confirmation. There are some more regular expressions I want to make sure work, for example /\d, the regular expression matching / and then a single digit character. Here is the latest version producing uncompilable typescript on that in the playground.

I think there should be a handful of tests containing regular expressions both escaping and not escaping different slashes and using character groups etc. I also think part of the test condition should be making sure the resultant code compiles.

mjperrone avatar Aug 18 '23 22:08 mjperrone

my bad, shouldnt have merged this PR initially. I had to merge cause it was blocking the main changeset PR in the meantime*

thought you were done after this message but I misnterpreted:

I haven't used changeset before, but I attempted to do that.

astahmer avatar Aug 18 '23 22:08 astahmer

I haven't had time to go back to this, but I'm confident that there is still a bug in the current version around this behavior.

mjperrone avatar Sep 01 '23 19:09 mjperrone

With the current version (1.11.1), this particular pattern in the spec is incorrectly generated (the escapes are not added to the generated code):

Pattern in spec: ^(https:\/\/)([^\/]+)\/([^\/]+)\/([^\/]+).pdf$ Pattern in generated code: /^(https://)([^/]+)/([^/]+)/([^/]+).pdf$/

prabhpreet avatar Sep 23 '23 13:09 prabhpreet

@astahmer would you be able to fix this bug with regular expression translation?

mjperrone avatar Oct 06 '23 16:10 mjperrone

@astahmer I spent a good amount of time trying to understand what's going on and in the end I corrected the test and reverted the change you merged: https://github.com/astahmer/openapi-zod-client/pull/288

I think this PR is a better outcome for most user situations and in the description and in-line comments I describe what's going on and what happened. String escaping is tedious to think about carefully!

mjperrone avatar Apr 16 '24 23:04 mjperrone