Repository navigation
refactor(validation): own the better-errors formatter - #77
Merged
Merged
Conversation
The JSON Schema provider formats Ajv errors with a port of @stoplight/better-ajv-errors@1.0.3, kept in providers/better-errors, instead of the package. The package declares an ajv peer, which npm installs, so it kept Ajv in the dependency tree, and it brought jsonpointer and leven with it. It leaves the optional dependencies. The output is unchanged. JSON Pointers are resolved with @swaggerexpert/json-pointer, the implementation under @speclynx/apidom-json-pointer, now a direct dependency; it was already installed, so the tree does not grow. It returns undefined wherever jsonpointer.get did, except for a member inherited from Object.prototype or the length of an array, paths Ajv never reports. The distance leven computed comes from a Levenshtein function written here. @stoplight/better-ajv-errors stays as an exact-pinned devDependency. A new test validates every fixture against every embedded schema and expects the port to return what the original returns, suggestions included. The provider drops suggestions, so the baselines cannot see the pointer lookup that feeds them. The browser bundle's notices keep crediting the package through additionalModules, which now take package names, and NOTICE records the port.
The better-ajv-errors entry moves the ApiDOM attribution in NOTICE from lines 55 and 57 to 63 and 65, and the rule that allows it in scripts/retired-identifiers-allow.txt names the lines.
There was a problem hiding this comment.
🟡 Changes recommended
The distance implementation performs avoidable quadratic work and uses oversized working arrays for long enum strings during synchronous validation.
1 open finding
What changed in this PR
Replaces the external better-errors formatter with a local port, reducing published dependencies ahead of Ajv removal.
Changes:
- Adds local error formatting, JSON Pointer lookup, and Levenshtein distance helpers.
- Adds upstream-parity and helper tests.
- Updates dependencies and third-party attribution.
| File | Description |
|---|---|
| scripts/third-party-notices.js | Resolves installed packages for attribution. |
| scripts/retired-identifiers-allow.txt | Adjusts NOTICE line references. |
| packages/api-languageservice/test/better-errors.ts | Adds parity and helper tests. |
| packages/api-languageservice/src/services/validation/providers/json-schema-validation-provider.ts | Uses the local formatter. |
| packages/api-languageservice/src/services/validation/providers/better-errors/levenshtein.ts | Implements edit distance. |
| packages/api-languageservice/src/services/validation/providers/better-errors/json-pointer.ts | Wraps JSON Pointer evaluation. |
| packages/api-languageservice/src/services/validation/providers/better-errors/index.ts | Implements the formatter port. |
| packages/api-languageservice/package.json | Updates runtime and development dependencies. |
| packages/api-languageservice/config/webpack/browser.config.js | Includes port attribution in notices. |
| package-lock.json | Synchronizes dependency classifications. |
| NOTICE | Credits the formatter’s upstream source. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The Levenshtein function that replaced leven filled a row as long as its second argument for every character of its first, allocating a new row each time. leven returned at once for equal strings, trimmed the prefix and suffix both strings share, and kept one row sized to the shorter string. An enum error's suggestion compares every allowed value with the document's value, which can be long: against a 100,000-character value one comparison took 17.5 ms and 9.3 MB, and two 2,001-character strings differing only at the end took 30 ms. The function now takes the same three steps, and those cases take 1.5 ms with 0.3 MB, and no measurable time. Distances are unchanged. A new test compares the function with the full table of edit distances on 2,000 seeded random pairs that share prefixes and suffixes, and it agreed with leven on 200,000 such pairs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is the fourth step of the plan to remove Ajv and the components flagged by the dependency policy scan from the published dependency tree. It removes
@stoplight/better-ajv-errors@1.0.3and, with it,leven@3.1.0(which the scan reports as older than five years) andjsonpointer@5.0.1. It is also a prerequisite for dropping Ajv in the next step: the package declares anajvpeer dependency, which npm installs automatically, so Ajv could not leave the tree while it stayed.The change
The JSON Schema provider formats Ajv's errors with a port of
@stoplight/better-ajv-errors@1.0.3(Apache-2.0) inproviders/better-errors/, and the package leavesoptionalDependencies. The port replaces the original's class per keyword with functions and drops the unusedschemaargument and thepropertyPathoption, which the provider always passed empty, so it is called asbetterAjvErrors(ajvErrors, document). Two quirks of the original are kept on purpose, with comments, because they decide which errors are merged and which are filtered: the instance-path regex cuts a segment at any character outside[\w$_-], so~1escapes can make different paths share a node, and anenum-only node is dropped whenever it has any sibling, because the original tested a list that is always truthy. A missing Ajvmessagebecomes''instead of crashing; Ajv always sets it with our options.The original resolved JSON Pointers with
jsonpointer, only to compute the "Did you mean" suggestion forenumerrors. The port uses@swaggerexpert/json-pointer(^3.0.2), the implementation underneath@speclynx/apidom-json-pointer, which becomes a direct dependency; it was already installed, so the tree does not grow.@speclynx/apidom-json-pointeritself cannot read the plain JSON document Ajv validated, because itsevaluatealways uses the ApiDOM realm. The smallgetByPointerwrapper returnsundefinedwhereverjsonpointer.getdid, with two exceptions that Ajv never produces: a member inherited fromObject.prototype(for example/constructoron an object without that key) and thelengthof an array.levenis replaced by a Levenshtein distance written here, over UTF-16 code units aslevenmeasures, so suggestions stay the same. Likeleven, it returns at once for equal strings, trims the prefix and suffix both strings share, and computes a single row sized to the shorter string, because the document value an enum error compares against can be long: the third commit restores these steps after review, taking one comparison against a 100,000-character value from 17.5 ms and 9.3 MB to 1.5 ms and 0.3 MB.@stoplight/better-ajv-errorsstays as an exact-pinned devDependency, as the oracle of the parity test below and as the source of its licence in the notices.scripts/third-party-notices.jsnow takesadditionalModulesas package names and finds each installed copy by walking upnode_modules, because the package does not export itspackage.json;dist/THIRD-PARTY-NOTICES.txtkeeps crediting it as the origin of the port, and NOTICE has an entry for it. The notices inventory goes from 59 to 57 packages asjsonpointerandlevenleave.Behaviour
The output is unchanged. The provider passes on better-errors'
errortext but drops itssuggestion, so the recorded baselines cannot see the pointer lookup that feeds suggestions;test/better-errors.tscovers it instead. It validates every parsable document undertest/fixturesagainst every one of the 15 embedded schemas, not only the one for its version, which produces errors of every shape the schemas can report, and expects the port to return exactly what the original returns, suggestions included. A targeted case pins the two suggestions of the OpenAPI 3.0 baseline document, one of them through the escaped pointer/paths/~1pets/get/parameters/0/in, and unit tests pingetByPointer(root, missing keys, index edge cases,-, traversal throughnulland primitives,~0and~1, own__proto__andconstructorkeys, and the two differences above) and the Levenshtein distance, which is also compared with the full table of edit distances on 2,000 seeded random pairs that share prefixes and suffixes.Testing
npm testpasses with 555 tests, and every recorded baseline is unchanged. Lint (no new warnings), type-check,npm run buildand./scripts/check-package-contract.shpass. In a scratch project that installs the packed package,@stoplight/better-ajv-errors,jsonpointer,levenandjson-source-mapare absent and@swaggerexpert/json-pointeris a single deduplicated copy;ajvandrequire-from-stringremain until the next step, which ships precompiled validators.This PR touches the same
devDependenciesblock and lockfile as #76, so whichever of the two lands second needs a small rebase.