Repository navigation
fix: registerAjvFormats() never registered any format - #295
Conversation
d60d57a to
0c16fa3
Compare
2bc8113 to
0c16fa3
Compare
0c16fa3 to
0adb0d7
Compare
Tony133
left a comment
There was a problem hiding this comment.
While checking this I noticed that on main registerAjvFormats() doesn't actually register anything: rawFormats is the plugin function, so Object.entries() only gives get and default and Format.Set is never reached. The tests pass because TypeBox has its own built-ins. So this PR is really a fix, and it changes behavior: ajv-formats now overrides 17 of TypeBox's formats and adds 9. For example url starts rejecting http://localhost:3000 and email rejects user@localhost, while date-time starts accepting a space separator and uuid the urn:uuid: prefix.
Keep in mind this package ships both CJS and ESM builds, and the ajv-formats interop is the part that behaves differently between the two, so that's where I'd be most careful.
A few things before merging:
- rename to something like
fix: registerAjvFormats() never registered any formatand mention the behavior change in the description, it should show up in the changelog - remove
int32,int64,float,double: they're number formats, TypeBox applies them to strings, so int32/int64 fail on everything and float/double pass everything - have the CJS test actually call
registerAjvFormats(), see inline - revert the formatting-only changes, see inline
Also worth noting: 100% wasn't reachable on main without touching the code, Format.Set was dead. So the fix was necessary, but the ESM test migration, the formatting and the extra branches in getFormatValidator (which then need the mock test) weren't.
I'd release this as a minor.
Hey @Tony133 let me know what do you think now 🙏 |
Tony133
left a comment
There was a problem hiding this comment.
LGTM!
Once merged, let's release this as a minor 🚀
registerAjvFormats()previously iterated the ajv-formats plugin object's properties instead of its format definitions, so it registered nothing. Existing validation tests passed because TypeBox already provides built-in formats.This fixes the helper to register 22 string formats using ajv-formats' full validation mode. It excludes
int32,int64,float, anddouble, whose numeric validators do not belong in TypeBox's string-format registry. The existing defensive default-export fallback is preserved for CJS, ESM, and bundler interoperability, with a coverage exclusion limited to that fallback. Validator normalization handles the real format definitions without the speculative error branch or its mock-only test. Thepasswordandbinaryformats share one always-true validator across registrations.Calling the helper replaces existing validators with the same names. Observable changes include rejecting
http://localhost:3000as aurlanduser@localhostas anemail, while accepting a space separator indate-timeand theurn:uuid:prefix inuuid. The README now documents these changes and corrects the helper import and the description of TypeBox's built-in formats.The complete runtime suite is shared between separate CJS and ESM entrypoints. c8 includes both builds and unexecuted JavaScript files, and requires 100% source coverage. Build source maps allow coverage to measure the original TypeScript rather than compiler-generated interoperability helpers. Unrelated formatting and dependency-order changes have been reverted.
Validation
npm run lintnpm test: both builds, package checks, 38 runtime tests, 20 type assertions, and 100% statements, branches, functions, and lines.Proposed release note
Fix
registerAjvFormats()so it registers AJV string-format validators. Calling the helper now overrides matching TypeBox defaults and can change accepted inputs, including URL, email, date-time, and UUID validation. Numeric formats are excluded. Recommend a minor release because the previously ineffective helper now changes runtime validation behavior.