Closed Spappz closed 2 months ago
Thanks for opening an issue!
I could certainly be open to having an option to try and correct it. Something like 'patternStrategy': 'escape' | 'preserve' | 'transform' where 'preserve' would still be the default behavior and 'transform' would do "its best" to handle compatibility issues be it from flags or whatever.
I'm a bit worried about how deep this rabbit hole goes though. I don't want to start introducing dependencies, and that includes adding enough code for a specific issue to warrant one.
You could certainly take a stab at it if you want, but just FYI, v4 is around the corner (late summer maybe) and will introduce a more granular lifecycle hook system specifically to let "edgy cases" like this be easier to handle in user land.
Thanks you the quick response!
I'm a bit worried about how deep this rabbit hole goes though. I don't want to start introducing dependencies, and that includes adding enough code for a specific issue to warrant one.
Completely reasonable.
Out of all the flags, s
seems trivial to implement. m
might be more involved, though i expect i
to be the worst. That said, unless I'm severely underestimating the number of special cases hidden in the specification, handling i
shouldn't take more than a couple dozen lines? Overall I don't think it'd require a new package dependency or monolithic amount of code to make a simple best-attempt, though I fully understand the wariness of maintenance debt.
v4 is around the corner
If something Big™ is coming soon, I'm happy leaving it be for now (I can fix my regexes to be case-insensitive without needing flags, in situ). Could a point about this be added to the Known Issues section of the README in the meantime? Happy to PR that too if it makes anything easier 😏
If something Big™ is coming soon, I'm happy leaving it be for now (I can fix my regexes to be case-insensitive without needing flags, in situ).
Actually @Spappz this might be a good time to play around with something more involved than a known issue warning. If it gets unwieldy we can just purge it in the next major.
A PR would be welcome :)
Minimal reproducible example
You can cut things short once
jsonSchema
is defined in all honesty—just inspect thepattern
. I'm using CommonJS so you can try it out in the Node REPL.The problem
The error seems to have two origins. Firstly, and most basically,
addPattern
just ignores RegExp flags, and only takes asvalue
the RegExpsource
string.Secondly, however, is that JSON Schema just doesn't seem to have any defined way to actually reproduce JS RegExp flags.
The solution
I'm not sure if there's a nice, trivial solution to this. A possible avenue, at least for case-insensitivity (in which I'm naturally most interested), is to 'correct' the regex in transit. You'd want to produce transformations like this, probably necessitating some horrid hackery in the process:
Admittedly very messy.
Bottom line
On the face of it, you could wontfix this and just tell users to construct their Zod regexes to be case-insensitive by design, without relying on the flag. However, that should definitely appear in the documentation as a limitation of JSON Schema/this module!
If the above hack is suitable to you, though, I'm happy writing a PR to implement it. I don't think it'd require much difference than just modifying
addPattern
and how it's called?