Combine required and dependentRequired error messages by Sam-61s · Pull Request #160 · hyperjump-io/json-schema-errors

Sam-61s · 2026-02-03T15:54:43Z

When required and dependentRequired fail at the same instance location, they previously produced multiple duplicate messages. This change consolidates those failures into a single required-message, combining and deduplicating missing properties while preserving schema locations.

Adds test coverage for the original issue scenario and keeps existing dependentRequired and dependency behavior unchanged.

Fixes #134

jdesrosiers

Try again. You should end up with one error handler that handles required, dependentRequired, and the array form of dependencies. There doesn't need to be a separate error handler for each keyword. They're defined separately specifically for that reason.

Sam-61s · 2026-02-04T01:12:25Z

Thanks for the clarification. I plan to unify required, dependentRequired, and the array-form of dependencies into required.js, while keeping schema-form dependencies handled separately through normal nested validation.

The unified handler will consolidate missing properties into a single required-message, deduplicate properties, and preserve schemaLocations. Since dependentRequired will be handled by the unified handler, I also plan to remove the separate dependentRequired.js handler.

Does that match your expectation?

jdesrosiers · 2026-02-04T05:58:09Z

Yep. That sounds right.

Sam-61s · 2026-02-06T12:06:54Z

I’ve updated the PR based on your guidance. There is now a single unified error handler in required.js that handles required, dependentRequired, and the array form of dependencies.
Could you please take another look when you have a moment? Thanks!

jdesrosiers

Looks good so far. I left a few comments inline, but there's one more thing I'd like you to update in general. When contributing to project, you should try to match the code style used in the project. The way you did it isn't wrong or bad, but it looks significantly different than the rest of the error handlers. It can make it harder to review when the patterns are different. So, I'd like you to rework this a bit so it looks more like other error handlers in the repo. This includes the JSON files. Especially avoid making a lot of unnecessary whitespace changes (like in dependencies.json).

jdesrosiers · 2026-02-06T21:05:11Z

src/error-handlers/required.js

+    /** @type {string[]} */
+    const missingProperties = [...allMissingRequired].sort();
+
+    /** @type {string[]} */
+    const locations = [...allSchemaLocations].sort();


No need to sort. Order doesn't matter.

jdesrosiers · 2026-02-06T21:07:51Z

src/error-handlers/required.js

-  /** @type ErrorObject[] */
-  const errors = [];
+  const allMissingRequired = new Set();
+  const allSchemaLocations = new Set();


Does this need to be a Set? I'm pretty sure it's not possible to have the same schema location come up twice.

jdesrosiers · 2026-02-06T21:10:40Z

src/normalization-handlers/draft-04/dependencies.js

-      return outputs;
+      return undefined;
    }

    for (const [propertyName, dependency] of dependencies) {
-      if (!Instance.has(propertyName, instance) || typeof dependency !== "string") {
+      if (!Instance.has(propertyName, instance)) {
        continue;
      }
-
-      outputs.push(evaluateSchema(dependency, instance, context));
+      if (typeof dependency === "string") {
+        outputs.push(evaluateSchema(dependency, instance, context));
+      }
    }
-
-    return outputs;
+    return outputs.length > 0 ? outputs : undefined;


I don't think the normalization handler needs to change. What's the purpose of these changes?

jdesrosiers · 2026-02-06T21:11:36Z

src/test-suite/tests/required.json

          "messageParams": {
            "count": 1,
-            "required": { "or": ["bar"] }
+            "required": { "and": ["bar"] }


Good catch! That definitely should have been an "and".

jdesrosiers · 2026-02-06T21:12:25Z

src/test-suite/tests/required.json

    },
+    {
+      "description": "combined required and dependentRequired",
+      "compatibility": ">=2019",


The >= isn't needed.

Suggested change

"compatibility": ">=2019",

"compatibility": "2019",

Copilot

Pull request overview

This PR consolidates validation errors that previously produced multiple required-message entries when required and dependentRequired (and some dependency cases) fail at the same instance location, deduplicating the missing property list while preserving schema locations.

Changes:

Update required error handling to aggregate missing properties across required, dependentRequired, and array-form draft-04 dependencies into a single required-message.
Remove the standalone dependentRequired error handler and remove array-form dependencies required-message generation from the draft-04 dependencies error handler.
Update/expand the JSON test-suite expectations to cover the consolidated-message behavior (including the reported issue scenario).

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/test-suite/tests/required.json	Adjusts existing required expectations and adds a new combined required+dependentRequired test case.
src/test-suite/tests/dependencies.json	Updates expected errors to reflect consolidated required-message behavior for dependentRequired/array-form dependencies and some dependentSchemas cases.
src/normalization-handlers/draft-04/dependencies.js	Minor cleanup (removes blank lines) in dependencies normalization handler.
src/index.js	Unregisters the removed dependentRequired error handler.
src/error-handlers/required.js	Implements combined missing-property aggregation across required/dependentRequired/dependencies.
src/error-handlers/draft-04/dependencies.js	Removes array-form dependency required-message generation; keeps dependent-schema traversal behavior.
src/error-handlers/dependentRequired.js	Deletes the standalone dependentRequired error handler (logic moved into required handler).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-07T01:46:06Z

src/error-handlers/required.js

+  const allMissingRequired = new Set();
+  const allSchemaLocations = [];

-  for (const schemaLocation in normalizedErrors["https://json-schema.org/keyword/required"]) {
-    if (normalizedErrors["https://json-schema.org/keyword/required"][schemaLocation]) {
-      continue;
+  const requiredErrors = normalizedErrors["https://json-schema.org/keyword/required"];
+  if (requiredErrors) {
+    for (const schemaLocation in requiredErrors) {
+      if (requiredErrors[schemaLocation] === false) {
+        allSchemaLocations.push(schemaLocation);
+        const keyword = await getSchema(schemaLocation);
+        const required = /** @type string[] */ (Schema.value(keyword));
+
+        for (const propertyName of required) {
+          if (!Instance.has(propertyName, instance)) {
+            allMissingRequired.add(propertyName);
+          }
+        }
+      }
    }
+  }

-    const keyword = await getSchema(schemaLocation);
-    const required = /** @type string[] */ (Schema.value(keyword));
+  const dependentRequiredErrors = normalizedErrors["https://json-schema.org/keyword/dependentRequired"];
+  if (dependentRequiredErrors) {
+    for (const schemaLocation in dependentRequiredErrors) {
+      if (dependentRequiredErrors[schemaLocation] === false) {
+        allSchemaLocations.push(schemaLocation);
+        const keyword = await getSchema(schemaLocation);
+        const dependentRequired = /** @type Record<string, string[]> */ (Schema.value(keyword));

-    const missingRequired = [];
-    for (const propertyName of required) {
-      if (!Instance.has(propertyName, instance)) {
-        missingRequired.push(propertyName);
+        for (const propertyName in dependentRequired) {
+          if (Instance.has(propertyName, instance)) {
+            for (const requiredPropertyName of dependentRequired[propertyName]) {
+              if (!Instance.has(requiredPropertyName, instance)) {
+                allMissingRequired.add(requiredPropertyName);
+              }
+            }
+          }
+        }
      }
    }
+  }
+
+  const dependenciesErrors = normalizedErrors["https://json-schema.org/keyword/draft-04/dependencies"];
+  if (dependenciesErrors) {
+    for (const schemaLocation in dependenciesErrors) {
+      const dependencyValue = dependenciesErrors[schemaLocation];
+      if (dependencyValue === false || Array.isArray(dependencyValue)) {
+        const keyword = await getSchema(schemaLocation);
+        const dependencies = /** @type Record<string, string[] | object> */ (Schema.value(keyword));
+
+        let hasArrayFormDependencies = false;
+        for (const propertyName in dependencies) {
+          if (Instance.has(propertyName, instance)) {
+            const dependency = dependencies[propertyName];
+            if (Array.isArray(dependency)) {
+              hasArrayFormDependencies = true;
+              const dependencyArray = /** @type {string[]} */ (dependency);
+              for (const requiredPropertyName of dependencyArray) {
+                if (!Instance.has(requiredPropertyName, instance)) {
+                  allMissingRequired.add(requiredPropertyName);
+                }
+              }
+            }
+          }
+        }
+
+        if (hasArrayFormDependencies) {
+          allSchemaLocations.push(schemaLocation);
+        }
+      }
+    }
+  }
+
+  if (allMissingRequired.size > 0) {
+    /** @type {string[]} */
+    const missingProperties = [...allMissingRequired].sort();
+    /** @type {string[]} */
+    const locations = [...allSchemaLocations].sort();

-    errors.push({
-      message: localization.getRequiredErrorMessage(missingRequired),
+    return [{
+      message: localization.getRequiredErrorMessage(missingProperties),
      instanceLocation: Instance.uri(instance),
-      schemaLocations: [schemaLocation]
-    });
+      schemaLocations: locations
+    }];
  }

-  return errors;
+  return [];


The updated handler now aggregates all required failures for an instance into a single required-message (and also folds dependentRequired and array-form dependencies into that same message). This is a broader behavior change than “when required and dependentRequired fail at the same instance location”, and it will also collapse multiple distinct required keywords (e.g., via allOf or dependentSchemas) that previously produced separate error objects. If that broader consolidation is intended, it should be explicitly called out (and treated as a potentially breaking change for consumers who rely on per-keyword errors); otherwise, scope the merge logic to only combine required with dependentRequired/dependencies at the same instance location and keep separate required keyword failures as separate error objects.

combined required and dependentRequired error messages

7258839

jdesrosiers requested changes Feb 4, 2026

View reviewed changes

unified error handler for each keyword

b2b2771

jdesrosiers requested changes Feb 6, 2026

View reviewed changes

code style is fixed

7773c0f

Copilot AI review requested due to automatic review settings February 7, 2026 01:41

Copilot started reviewing on behalf of Sam-61s February 7, 2026 01:42 View session

Copilot AI reviewed Feb 7, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Combine required and dependentRequired error messages#160

Combine required and dependentRequired error messages#160
Sam-61s wants to merge 3 commits intohyperjump-io:mainfrom
Sam-61s:feat/combine-required-message

Sam-61s commented Feb 3, 2026

Uh oh!

jdesrosiers left a comment

Uh oh!

Sam-61s commented Feb 4, 2026

Uh oh!

jdesrosiers commented Feb 4, 2026

Uh oh!

Sam-61s commented Feb 6, 2026

Uh oh!

jdesrosiers left a comment

Uh oh!

jdesrosiers Feb 6, 2026

Uh oh!

jdesrosiers Feb 6, 2026

Uh oh!

jdesrosiers Feb 6, 2026

Uh oh!

jdesrosiers Feb 6, 2026

Uh oh!

jdesrosiers Feb 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Sam-61s commented Feb 3, 2026

Uh oh!

jdesrosiers left a comment

Choose a reason for hiding this comment

Uh oh!

Sam-61s commented Feb 4, 2026

Uh oh!

jdesrosiers commented Feb 4, 2026

Uh oh!

Sam-61s commented Feb 6, 2026

Uh oh!

jdesrosiers left a comment

Choose a reason for hiding this comment

Uh oh!

jdesrosiers Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

jdesrosiers Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

jdesrosiers Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

jdesrosiers Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

jdesrosiers Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants