Repository: mozilla-services/react-jsonschema-form
Pull request #1048 (2018-10-13 06:31:50)

Validation errors when using refs in dependencies
### Reasons for making this change

I understand that there is no fully support for `oneOf`, nevertheless, on the README it says that except for a special case where we can use oneOf in schema dependencies.
I search on the issues and couldn't find something related. My issue is when inside the oneOf I have $refs. Looks like it doesn't match any of the options. I attached a shared playground link showing it.

### Steps to Reproduce

1. Open the [shared playground example](https://mozilla-services.github.io/react-jsonschema-form/#eyJmb3JtRGF0YSI6eyJwZXJzb24iOnsiRG8geW91IGhhdmUgYW55IHBldHM/IjoiTm8ifX0sInNjaGVtYSI6eyJ0aXRsZSI6IlNjaGVtYSBkZXBlbmRlbmNpZXMiLCJ0eXBlIjoib2JqZWN0IiwiZGVmaW5pdGlvbnMiOnsib3B0aW9uMSI6eyJwcm9wZXJ0aWVzIjp7IkRvIHlvdSBoYXZlIGFueSBwZXRzPyI6eyJlbnVtIjpbIk5vIl19fX0sIm9wdGlvbjIiOnsicHJvcGVydGllcyI6eyJEbyB5b3UgaGF2ZSBhbnkgcGV0cz8iOnsiZW51bSI6WyJZZXM6IE9uZSJdfSwiSG93IG9sZCBpcyB5b3VyIHBldD8iOnsidHlwZSI6Im51bWJlciJ9fSwicmVxdWlyZWQiOlsiSG93IG9sZCBpcyB5b3VyIHBldD8iXX0sIm9wdGlvbjMiOnsicHJvcGVydGllcyI6eyJEbyB5b3UgaGF2ZSBhbnkgcGV0cz8iOnsiZW51bSI6WyJZZXM6IE1vcmUgdGhhbiBvbmUiXX0sIkRvIHlvdSB3YW50IHRvIGdldCByaWQgb2YgYW55PyI6eyJ0eXBlIjoiYm9vbGVhbiJ9fSwicmVxdWlyZWQiOlsiRG8geW91IHdhbnQgdG8gZ2V0IHJpZCBvZiBhbnk/Il19fSwicHJvcGVydGllcyI6eyJwZXJzb24iOnsidGl0bGUiOiJQZXJzb24iLCJ0eXBlIjoib2JqZWN0IiwicHJvcGVydGllcyI6eyJEbyB5b3UgaGF2ZSBhbnkgcGV0cz8iOnsidHlwZSI6InN0cmluZyIsImVudW0iOlsiTm8iLCJZZXM6IE9uZSIsIlllczogTW9yZSB0aGFuIG9uZSJdLCJkZWZhdWx0IjoiTm8ifX0sInJlcXVpcmVkIjpbIkRvIHlvdSBoYXZlIGFueSBwZXRzPyJdLCJkZXBlbmRlbmNpZXMiOnsiRG8geW91IGhhdmUgYW55IHBldHM/Ijp7Im9uZU9mIjpbeyIkcmVmIjoiIy9kZWZpbml0aW9ucy9vcHRpb24xIn0seyIkcmVmIjoiIy9kZWZpbml0aW9ucy9vcHRpb24yIn0seyIkcmVmIjoiIy9kZWZpbml0aW9ucy9vcHRpb24zIn1dfX19fX0sInVpU2NoZW1hIjp7fX0=)
2. Select any option on the **Do you have any pets?** question.

#### Expected behavior

For this example I took the working example and just moved the options to the definitions and add the refs. So I would expect to work as usual and is not.

#### Actual behavior

When moving the possible options to the definitions section. It does not match any of them showing the errors:
- should be equal to one of the allowed values
- should match exactly one schema in oneOf

### Version
Playground

### Checklist

* [ ] **I'm updating documentation**
  - [ ] I've checked the rendering of the Markdown text I've added
  - [ ] If I'm adding a new section, I've updated the Table of Content
* [ ] **I'm adding or updating code**
  - [x] I've added and/or updated tests
  - [x] I've updated docs if needed
  - [x] I've run `npm run cs-format` on my branch to conform my code to [prettier](https://github.com/prettier/prettier) coding style
* [ ] **I'm adding a new feature**
  - [ ] I've updated the playground with an example use of the feature


glasserc: Refs #1044.

I'll try to look at this later tonight.
glasserc: This is a very clean PR and it was really easy to read, thanks!

I think it would be good to add a test that this actually fixes the issue, namely that we resolve references in `oneOf`.

I also wonder if it's even really worth extracting a function for `resolvesReferences`, since it's basically `map` + `resolveReference`. Could we maybe move the `if` statement checking for `$ref` inside `resolveReference` (and just return the `schema` if there isn't a `$ref`) and then just use `map`? We could check whether `oneOf` is an array just once somewhere else. (It has to be, according to the JSON Schema spec.)
krilbert: Thank you :D
I did add a test proving that refs are being resolved inside oneOf `"with $ref in oneOf"` next to `with $ref in dependency`.
To be honest I tried to touch the less amount of code. If I understand correctly you mean moving the resolve logic inside the `resolveSchema` right? Let me take a look
glasserc: Ah, thanks for fixing the test! No, let's not resolve `oneOf` in `resolveSchema`, I think that makes `resolveSchema` too complicated. Let's just have `withDependentSchema` call `resolveReference` on every schema in `oneOf`, unconditionally. (This means moving the conditional into `resolveReference`.) If you're worried that `oneOf` isn't an array, we can leave the check wherever you like -- at the top of `withDependentSchema` maybe, or even in `resolveSchema`.
krilbert: The issue is that if we only check under `withDependentSchema` it will only resolve when having `dependencies` am I right? I'll take a look at a chance 😳 
glasserc: Well, the original issue is only about `oneOf` inside of dependencies, so I think that's OK.
krilbert: I updated the test to reflect the bug fix

Patch:
diff --git a/src/utils.js b/src/utils.js
index 8aeae8b9a..25cfe6ad3 100644
--- a/src/utils.js
+++ b/src/utils.js
@@ -455,16 +455,7 @@ export function stubExistingAdditionalProperties(
 
 export function resolveSchema(schema, definitions = {}, formData = {}) {
   if (schema.hasOwnProperty("$ref")) {
-    // Retrieve the referenced schema definition.
-    const $refSchema = findSchemaDefinition(schema.$ref, definitions);
-    // Drop the $ref property of the source schema.
-    const { $ref, ...localSchema } = schema;
-    // Update referenced schema definition with local schema properties.
-    return retrieveSchema(
-      { ...$refSchema, ...localSchema },
-      definitions,
-      formData
-    );
+    return resolveReference(schema, definitions, formData);
   } else if (schema.hasOwnProperty("dependencies")) {
     const resolvedSchema = resolveDependencies(schema, definitions, formData);
     return retrieveSchema(resolvedSchema, definitions, formData);
@@ -474,6 +465,19 @@ export function resolveSchema(schema, definitions = {}, formData = {}) {
   }
 }
 
+function resolveReference(schema, definitions, formData) {
+  // Retrieve the referenced schema definition.
+  const $refSchema = findSchemaDefinition(schema.$ref, definitions);
+  // Drop the $ref property of the source schema.
+  const { $ref, ...localSchema } = schema;
+  // Update referenced schema definition with local schema properties.
+  return retrieveSchema(
+    { ...$refSchema, ...localSchema },
+    definitions,
+    formData
+  );
+}
+
 export function retrieveSchema(schema, definitions = {}, formData = {}) {
   const resolvedSchema = resolveSchema(schema, definitions, formData);
   const hasAdditionalProperties =
@@ -537,15 +541,26 @@ function withDependentSchema(
     formData
   );
   schema = mergeSchemas(schema, dependentSchema);
-  return oneOf === undefined
-    ? schema
-    : withExactlyOneSubschema(
-        schema,
-        definitions,
-        formData,
-        dependencyKey,
-        oneOf
-      );
+  // Since it does not contain oneOf, we return the original schema.
+  if (oneOf === undefined) {
+    return schema;
+  } else if (!Array.isArray(oneOf)) {
+    throw new Error(`invalid: it is some ${typeof oneOf} instead of an array`);
+  }
+  // Resolve $refs inside oneOf.
+  const resolvedOneOf = oneOf.map(
+    subschema =>
+      subschema.hasOwnProperty("$ref")
+        ? resolveReference(subschema, definitions, formData)
+        : subschema
+  );
+  return withExactlyOneSubschema(
+    schema,
+    definitions,
+    formData,
+    dependencyKey,
+    resolvedOneOf
+  );
 }
 
 function withExactlyOneSubschema(
@@ -555,11 +570,6 @@ function withExactlyOneSubschema(
   dependencyKey,
   oneOf
 ) {
-  if (!Array.isArray(oneOf)) {
-    throw new Error(
-      `invalid oneOf: it is some ${typeof oneOf} instead of an array`
-    );
-  }
   const validSubschemas = oneOf.filter(subschema => {
     if (!subschema.properties) {
       return false;


Test patch:
diff --git a/test/utils_test.js b/test/utils_test.js
index 24a3bf19e..fb688506c 100644
--- a/test/utils_test.js
+++ b/test/utils_test.js
@@ -688,6 +688,47 @@ describe("utils", () => {
             });
           });
         });
+
+        describe("with $ref in oneOf", () => {
+          it("should retrieve referenced schemas", () => {
+            const schema = {
+              type: "object",
+              properties: {
+                a: { enum: ["typeA", "typeB"] },
+              },
+              dependencies: {
+                a: {
+                  oneOf: [
+                    { $ref: "#/definitions/needsA" },
+                    { $ref: "#/definitions/needsB" },
+                  ],
+                },
+              },
+            };
+            const definitions = {
+              needsA: {
+                properties: {
+                  a: { enum: ["typeA"] },
+                  b: { type: "number" },
+                },
+              },
+              needsB: {
+                properties: {
+                  a: { enum: ["typeB"] },
+                  c: { type: "boolean" },
+                },
+              },
+            };
+            const formData = { a: "typeB" };
+            expect(retrieveSchema(schema, definitions, formData)).eql({
+              type: "object",
+              properties: {
+                a: { enum: ["typeA", "typeB"] },
+                c: { type: "boolean" },
+              },
+            });
+          });
+        });
       });
 
       describe("dynamic", () => {