Skip to content

feat(form): accept AJV 8 instancePath in formatErrors (dual-shape) - #78

Merged
hugomallet merged 2 commits into
masterfrom
feat/formaterrors-dual-shape
Aug 8, 2026
Merged

hugomallet merged 2 commits into
masterfrom
feat/formaterrors-dual-shape

Conversation

@hugomallet

Copy link
Copy Markdown
Collaborator

Why

First brick of the AJV 8 track: formatErrors becomes shape-agnostic so the future AJV 8 bump (separate PR, v1 branch) and a potential Standard Schema adapter only have to swap the validator, not the error plumbing. AJV 8 replaced dataPath (dot/bracket notation) with instancePath (JSON Pointer, RFC 6901).

What

  • formatErrors reads error.instancePath when it is a string, and falls back to the legacy error.dataPath otherwise (the active branch with the bundled AJV 6).
  • New exported helper pointerToFieldPath(pointer) — JSON Pointer to dot-separated field path, with RFC 6901 escape decoding (~1 before ~0, so ~01 correctly decodes to a literal ~1).
  • The required concatenation is now conditional (field ? field + '.' + missing : missing), removing the orphan-leading-dot dance at the root.
error input field (before) field (after)
dataPath: 'arr[0].input' arr.0.input arr.0.input (unchanged)
dataPath: '' + required input input input (unchanged)
instancePath: '/arr/0/input' n/a (undefined behavior) arr.0.input
instancePath: '' + required input n/a input
instancePath: '/a~01b' n/a a~1b

Non-breaking, proven

  • The 5 pre-existing dataPath tests are untouched (the test file diff is purely additive) and still pass — strict non-regression under AJV 6.
  • 11 new tests: mirror instancePath set (same cases as the dataPath set), pointer edge cases (root pointer, ~0/~1 escapes and their decoding order, required inside an array item), plus direct pointerToFieldPath unit tests with literal expectations.
  • Each new test was mutation-checked: swapping the ~0/~1 decoding order, dropping the slice(1), making the required concat unconditional, and weakening the typeof check to truthiness each make the corresponding tests fail for the expected reason.
  • Full suite: 113 tests before, 124 after, all green. yarn lint and yarn type-check green.

formatErrors now reads error.instancePath (JSON Pointer, RFC 6901 —
the AJV 8+ error shape) when present, and falls back to the legacy
error.dataPath (dot/bracket notation, AJV <= 6) otherwise. With the
bundled AJV 6 the behavior is strictly unchanged: the pre-existing
dataPath tests are kept untouched and prove the non-regression.

- new exported helper pointerToFieldPath(pointer): JSON Pointer ->
  dot-separated field path, with RFC 6901 escape decoding (~1 before
  ~0, so ~01 correctly yields a literal "~1")
- the required-error concatenation is now conditional, so the root
  path no longer relies on stripping an orphan leading dot
- mirror instancePath test set + pointer edge cases (root pointer,
  escapes, required inside arrays)

Groundwork for the AJV 8 upgrade and a future Standard Schema adapter.

@hugomallet hugomallet left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Tooled self-review (claude + review53 skill) — posted as comment, author and reviewer are the same account.

Verdict: LGTM, no correctness issue found. ESLint + tsc green on changed files, 124/124 tests pass, CI fully green (build + types React 16–19).

Verified beyond the suite:

  • RFC 6901 edges (tested locally): lone / → '', /a//b → a..b, /0 → 0, /~01 → ~1. All decode per the RFC; the / vs root-pointer collision (both → '') is a representational limit of dot paths, not a decoding bug.
  • dataPath equivalence: ran the old master implementation differentially against the new code on 8 dataPath cases, including dataPath: '[0]' + required — outputs strictly identical, bug-for-bug (both keep the pre-existing leading dot on root-array dataPath, .0.x).
  • Dispatch predicate: typeof instancePath === 'string' is the right choice — truthiness would misroute AJV 8's root ''.

Minor, non-blocking:

  1. CHANGELOG overstates the export. pointerToFieldPath is exported from helpers.js only; the package exports map exposes just dist/index.js, which does not re-export it, so it is not user-visible. Reword ("internal helper") or re-export from the public barrel if intended.
  2. Neither-shape error objects now always throw (formatted.dataPath.replace on undefined). Old code threw too, except required errors which produced garbage "undefined.x". Loud failure on degenerate input is fine — just noting the divergence.
  3. Inherited cross-shape quirk: root-array dataPath '[0]' → .0 (leading dot) vs instancePath '/0' → 0. Dies with the dataPath branch in PR 2 — no action.
  4. Optional: pin lone / and empty-segment (/a//b) behavior with two tests.

- treat an error carrying neither instancePath nor dataPath as
  pointing at the root instead of throwing (dataPath ?? '')
- CHANGELOG: pointerToFieldPath is internal (not reachable through
  the package exports map), reworded accordingly
- tests: degenerate no-path required error, lone-slash pointer '/'
  (documented root collision), empty mid-pointer segments '/a//b'
@hugomallet

Copy link
Copy Markdown
Collaborator Author

Review comments addressed in 889c461 (single round-trip):

  1. CHANGELOG — reworded: pointerToFieldPath is described as a new internal helper (groundwork for AJV 8), since it is not reachable through the package exports map. No public export added in this PR.
  2. Degenerate input — an error with neither instancePath nor dataPath no longer throws: the missing path is treated as the root (dataPath ?? ''). Test added ({keyword: 'required', params: {missingProperty: 'x'}} → field 'x', no throw); mutation-checked (dropping the ?? '' makes it fail with the exact TypeError).
  3. Optional pointer tests added — lone-slash '/' → '' (root collision documented in the test comment) and '/a//b' → 'a..b'; both mutation-checked (filtering empty segments / dropping slice(1) makes them fail).

The cross-shape [0] quirk is deliberately left for PR 2 as agreed.

Suite: 124 → 127 tests, all green; lint and type-check green.

@hugomallet
hugomallet merged commit 1e96295 into master Aug 8, 2026
5 checks passed
@hugomallet
hugomallet deleted the feat/formaterrors-dual-shape branch August 8, 2026 15:23
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.

1 participant