Skip to content

Remove ajv-formats-draft2019 #1411

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pasieronen wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Remove ajv-formats-draft2019 #1411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4d3ab6f
refactor: replace ajv-formats-draft2019 with custom implementations f…
pasieronen Apr 2, 2026
6f127bd
test: add test cases for idn-email validation
pasieronen Apr 7, 2026
eac25af
Merge branch 'main' into replace-ajv-formats-draft2019
jkowalleck Apr 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ Some features require optional peer dependencies — see `package.json` for vers
* Validation of JSON on _Node.js_ requires all of:
* [`ajv`](https://www.npmjs.com/package/ajv)
* [`ajv-formats`](https://www.npmjs.com/package/ajv-formats)
* [`ajv-formats-draft2019`](https://www.npmjs.com/package/ajv-formats-draft2019)
* Validation of XML on _Node.js_ requires any of:
* [`libxmljs2`](https://www.npmjs.com/package/libxmljs2)
* the system might need to meet the requirements for [`node-gyp`](https://github.com/TooTallNate/node-gyp#installation), in certain cases.
Expand Down
1 change: 0 additions & 1 deletion docs/install.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ See the shipped ``package.json`` for version constraints.
* Validation of JSON on _Node.js_ requires all of:
* `ajv <https://www.npmjs.com/package/ajv>`_
* `ajv-formats <https://www.npmjs.com/package/ajv-formats>`_
* `ajv-formats-draft2019 <https://www.npmjs.com/package/ajv-formats-draft2019>`_
* Validation of XML on _Node.js_ requires all of:
* `libxmljs2 <https://www.npmjs.com/package/libxmljs2>`_
* the system must meet the requirements for `node-gyp <https://github.com/TooTallNate/node-gyp#installation>`_
5 changes: 0 additions & 5 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@
"peerDependencies": {
"ajv": "^8.12.0",
"ajv-formats": "^3.0.1",
"ajv-formats-draft2019": "^1.6.1",
"libxmljs2": "^0.35||^0.37",
"xmlbuilder2": "^3.0.2||^4.0.0",
"packageurl-js": "*",
Expand All @@ -96,9 +95,6 @@
"ajv-formats": {
"optional": true
},
"ajv-formats-draft2019": {
"optional": true
},
"libxmljs2": {
"optional": true
},
Expand Down Expand Up @@ -127,7 +123,6 @@
"devDependencies": {
"ajv": "^8.12.0",
"ajv-formats": "^3.0.1",
"ajv-formats-draft2019": "^1.6.1",
"libxmljs2": "^0.35||^0.37",
"xmlbuilder2": "^3.0.2||^4.0.0",
"spdx-expression-parse": "^3.0.1||^4",
Expand Down
14 changes: 10 additions & 4 deletions src/_optPlug.node/__jsonValidators/ajv.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ import { readFile } from 'node:fs/promises'

import Ajv, { type Options as AjvOptions } from 'ajv'
import addFormats from 'ajv-formats'
/* @ts-expect-error TS7016 */
import addFormats2019 from 'ajv-formats-draft2019'

import type { ValidationError } from '../../validation/types'
import type { Functionality, Validator } from '../jsonValidator'
Expand Down Expand Up @@ -50,11 +48,19 @@ export default (async function (schemaPath: string, schemaMap: Record<string, st
/* eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- intended */
const ajv = new Ajv({ ...ajvOptions, schemas })
addFormats(ajv)
/* eslint-disable-next-line @typescript-eslint/no-unsafe-call -- intended */
addFormats2019(ajv, { formats: ['idn-email'] })

// there is just no working implementation for format "iri-reference": see https://github.com/luzlab/ajv-formats-draft2019/issues/22
ajv.addFormat('iri-reference', true)

// add idn-email format (was previously provided by ajv-formats-draft2019)
const emailValidator = ajv.compile({type: 'string', format: 'email'})
Copy link
Copy Markdown
Member

@jkowalleck jkowalleck Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since using our own implementations, we critically need tests for this!
please add test cases:

  • positive cases - like "this is expected to be treated as a valid 'idn-email'"
  • negative cases - like "this is expected to be treated as an invalid 'idn-email'"
  • edge cases - like UTF8 characters, quoted emails ("foo@bar"@baz) and waht not in the

Copy link
Copy Markdown
Author

@pasieronen pasieronen Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For anything containing only ASCII characters, we just reuse the implementation from ajv-formats (whose definition of what exactly is a valid email might not be identical to somebody else's definition - I don't think we want to write detailed tests for that).

But I agree we need something here to show the code works. I took the test cases I found in https://github.com/json-schema-org/JSON-Schema-Test-Suite/blob/e82bfdfa59c63cc74175d35fac81bb95e61db24b/tests/draft2019-09/optional/format/email.json and https://github.com/json-schema-org/JSON-Schema-Test-Suite/blob/e82bfdfa59c63cc74175d35fac81bb95e61db24b/tests/draft2019-09/optional/format/idn-email.json,
would those be enough?

Copy link
Copy Markdown
Member

@jkowalleck jkowalleck Apr 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you also add edge cases?
Things i can imagine:

  • empty email address (empty string) - shall be a negative
  • to be continued

here are other cases that might be woven in:

see also the test cases of the already used library for reference:

Copy link
Copy Markdown
Author

@pasieronen pasieronen Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! Especially the empty string is a good idea, added couple more at the same time (amended the same commit).

I also noticed that not even ajv-formats and ajv-formats-draft2019 agree on every input. For example, "John Doe"@example.com is valid according to the latter but not the former. That said, I don't expect this to be a problem (and we probably don't want too many tests for rare edge cases, since ajv-formats itself may also change over time).

ajv.addFormat('idn-email', {
type: 'string',
// syntax allows non-ASCII characters in places where 'x' would be allowed
// (don't attempt to validate exactly which Unicode characters are OK - too complex)
validate: x => emailValidator(x.replace(/[\u0080-\uffff]+/g, 'x'))
})

const validator = ajv.compile(schema)

return function (data: string): null | ValidationError {
Expand Down
2 changes: 1 addition & 1 deletion src/_optPlug.node/jsonValidator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export default opWrapper<Functionality>('JsonValidator', [
/* eslint-disable @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-return, @typescript-eslint/no-require-imports
-- needed */

['( ajv && ajv-formats && ajv-formats-draft2019 )', () => require('./__jsonValidators/ajv').default]
['( ajv && ajv-formats )', () => require('./__jsonValidators/ajv').default]
// ... add others here, pull-requests welcome!

/* eslint-enable @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-return, @typescript-eslint/no-require-imports */
Expand Down
45 changes: 45 additions & 0 deletions tests/functional/internals/OpPlug.node.jsonValidator.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,49 @@ suite('functional: internals: OpPlug.node.jsonValidator auto', () => {
const validator = await makeValidator(schemaPath, schemaMap)
assert.throws(() => { validator(brokenJson) })
})

// list of valid/invalid emails is from https://github.com/json-schema-org/JSON-Schema-Test-Suite
// (tests/draft2019-09/optional/format/email.json and idn-email.json)
// and https://github.com/luzlab/ajv-formats-draft2019 (index.test.js)

suite('accepts valid emails', () =>
[
'joe.bloggs@example.com',
'te~st@example.com',
'~test@example.com',
'test~@example.com',
'te.s.t@example.com',
'실례@실례.테스트',
'квіточка@пошта.укр',
'Dörte@Sörensen.example.com'
].forEach(validEmail => test(validEmail, async () => {
const validator = await makeValidator(schemaPath, schemaMap)
const validJson = JSON.stringify({
bomFormat: 'CycloneDX',
specVersion: '1.7',
metadata: { authors: [{ email: validEmail }] }
})
assert.strictEqual(validator(validJson), null)
}))
)

suite('rejects invalid emails', () =>
[
'2962',
'.test@example.com',
'test.@example.com',
'te..st@example.com',
'',
'johndoe',
'valid@example.com?asdf'
].forEach(invalidEmail => test(invalidEmail, async () => {
const validator = await makeValidator(schemaPath, schemaMap)
const invalidJson = JSON.stringify({
bomFormat: 'CycloneDX',
specVersion: '1.7',
metadata: { authors: [{ email: invalidEmail }] }
})
assert.notEqual(validator(invalidJson), null)
}))
)
})