Skip to content

fix(codegen): do not double escape slashes in regex constants - #5460

Open
kwy404 wants to merge 1 commit into
reduxjs:masterfrom
kwy404:fix/codegen-escaped-slash-pattern
Open

kwy404 wants to merge 1 commit into
reduxjs:masterfrom
kwy404:fix/codegen-escaped-slash-pattern

Conversation

@kwy404

@kwy404 kwy404 commented Oct 1, 2026

Copy link
Copy Markdown

Root cause: with outputRegexConstants: true, generateRegexConstantsForType escapes the schema pattern by replacing every / with \/ before wrapping it in a regex literal. That also hits slashes that are already escaped. A pattern like ^https?:\/\/[^\s]+$ (valid in JSON Schema, and what you get from any JS RegExp#source, since source always escapes /) becomes /^https?:\\/\\/[^\s]+$/. The \\ is read as an escaped backslash, so the next / closes the literal early, and prettier then throws SyntaxError: Invalid character while formatting the output, so the whole generation fails.

Fix: only escape bare slashes. The replace now matches escape pairs (a backslash plus the next character) and bare / together, and leaves the pairs untouched. Patterns without escaped slashes produce the same output as before.

Test: the website pattern in test/fixtures/petstore.yaml now uses the escaped form (petstore.json keeps the unescaped one, which the existing test still covers), and a new test in generateEndpoints.test.ts expects export const userWebsitePattern = /^https?:\/\/[^\s]+$/. Before the fix it fails (and so does the existing YAML regex test) with the SyntaxError above; after the fix all regex constants tests pass.

@codesandbox

codesandbox Bot commented Oct 1, 2026

Copy link
Copy Markdown

Review or Edit in CodeSandbox

Open the branch in Web Editor • VS Code • Insiders

Open Preview

@phryneas

phryneas commented Oct 3, 2026

Copy link
Copy Markdown
Member

You would only escape / in regular expressions if the regular expression is wrapped in / characters - which is not the case for the regular expressions in the openapi spec.

So if your Regexp was written like /foo\/bar/ it would need to be escaped, but not if it is written foo/bar.

=> I am a bit confused how you are hitting this in the first place, and I am not 100% convinced that this is always correct. It seems "harmless enough", but that shouldn't be the basis of our decision here tbh.

Can you show us any official documents that show escaped backslashes in OpenAPI definitions? This seems like a quirk of whatever you generate your spec with here tbh.

@kwy404

kwy404 commented Oct 3, 2026 •

Copy link
Copy Markdown
Author

Thanks for taking a look, that's a fair question.

To be upfront, I found this while reading the regex constants code rather than from a production spec, and then checked what common tools emit. Specs generated from Zod schemas are one real source: z.toJSONSchema() (and anything else that serializes a JS regex through RegExp.prototype.source) emits slashes escaped, because ECMA-262 requires source to escape /: new RegExp('a/b').source is a\/b. With Zod 4.6, z.string().regex(/^https?:\/\/[^\s]+$/) becomes "pattern": "^https?:\\/\\/[^\\s]+$" in the JSON, so the pattern string itself contains \/.

On the spec side, OpenAPI 3.0 says pattern follows the ECMA-262 regular expression dialect, and 3.1 defers to JSON Schema, which says the same. In that dialect \/ is a valid identity escape that matches /, with or without the u flag, so ^https?:\/\/ and ^https?:// describe the same pattern. I don't know of an official example that uses the escaped form, so I can't point you to one. It's valid input that real tools produce.

Today that input makes generation fail (the \\ closes the regex literal early and prettier throws), and the change only affects patterns that currently fail; every other pattern produces the same output as before. If you'd prefer a different approach, for example normalizing \/ to / first and then escaping every /, I'm happy to switch to that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants