Skip to content
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.

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

retrieveSchema API no longer resolves references from rootSchema #3761

Closed
1 task done
john-prillaman opened this issue Jul 12, 2023 · 11 comments · Fixed by #3895
Closed
1 task done

retrieveSchema API no longer resolves references from rootSchema #3761

john-prillaman opened this issue Jul 12, 2023 · 11 comments · Fixed by #3895
Assignees
Labels
bug p1 Important to fix soon utils Related to @rjsf/utils

Comments

@john-prillaman
Copy link

Prerequisites

What theme are you using?

core

What is your question?

In version 5.0.0-beta.11 of the rjsf utils package the exported retrieveSchema API could be used such that references in the provided schema could be resolved from definitions in the provided rootSchema. With the recent updates which include the retrieveSchemaInternal this functionality no longer works.
I understand that the schema form does not support resolving references from an external schema, however this functionality was available from this utils method and the interface and documentation suggests that it should allow this.
Will this functionality be available in future releases or was this never the intended use?

@john-prillaman john-prillaman added needs triage Initial label given, to be assigned correct labels and assigned question labels Jul 12, 2023
@heath-freenome
Copy link
Member

@john-prillaman Can you be more clear? The definition of retrieveSchema() API hasn't changed at since the initial 5.0.0-beta.1 release. It still takes a rootSchema as it's optional third parameter. Is there some sort of bug you are experiencing?

@heath-freenome heath-freenome added awaiting response and removed needs triage Initial label given, to be assigned correct labels and assigned labels Jul 12, 2023
@john-prillaman
Copy link
Author

In the 5.0.0-beta.1 references in the provided schema would be resolved from the given rootSchema. That functionality is no longer working.
For example a definition for a reference such as "$ref": "#/definitions/entity" must exists in the schema, where as in the beta version that definition could be resolved from the rootSchema and was not necessary to include that definition in the schema.

@heath-freenome
Copy link
Member

@john-prillaman Can you provide a reproducible example of this issue using codesandbox.io, jsfiddle or some other tool? As far as I know, passing rootSchema will allow the ref look ups just fine

@john-prillaman
Copy link
Author

john-prillaman commented Aug 4, 2023

I can work on using a tool but in the meantime this snippet of code should demonstrate the behavior.

const schema = {
    type: "object",
    properties: {
      entity: {
        $ref: "#/definitions/entity"
      }
    }
  };

  const rootSchema = {
    type: "object",
    definitions: {
      entity: {
        type: "string",
        title: "Entity"
      }
    }
  };
  const resolved = retrieveSchema(validator, schema, rootSchema)
  console.log(JSON.stringify(resolved));

Output using the beta version:
{"type":"object","properties":{"entity":{"type":"string","title":"Entity"}}}

Output using a newer version (seems to begin with the implementation of retrieveSchemaInternal):
{"type":"object","properties":{"entity":{"$ref":"#/definitions/entity"}}}

As you can see in my testing I find that in the second example the reference to the definition in the rootSchema has not been resolved.

@john-prillaman
Copy link
Author

john-prillaman commented Aug 11, 2023

If you need to see this in codesandbox I have an example of the previous snippet running here:
https://codesandbox.io/s/rjfs-utils-retrieveschema-smt37m?file=/src/App.js

Appears that the last version to provide expected results is @rjsf/utils@5.0.0-beta.14

@heath-freenome
Copy link
Member

@john-prillaman Thanks for the use case! I'll try to figure it out in the next week or so

@aiibe
Copy link

aiibe commented Sep 12, 2023

On version 5.13.0, the problem remains. I forked @john-prillaman code, and put definitions inside the schema itself.
https://codesandbox.io/s/rjfs-utils-retrieveschema-forked-7kghz7?file=/src/App.js

@heath-freenome heath-freenome added bug p1 Important to fix soon utils Related to @rjsf/utils and removed question awaiting response labels Sep 15, 2023
@heath-freenome heath-freenome self-assigned this Sep 15, 2023
@heath-freenome
Copy link
Member

@aiibe Thanks for pointing it out. My company has a slightly different issue that I'm hoping to fix at the beginning of October so I'll also fix this one

@john-prillaman
Copy link
Author

Awesome, thank you!

heath-freenome added a commit to heath-freenome/react-jsonschema-form that referenced this issue Oct 6, 2023
…d validator-ajv8

Fixes rjsf-team#3671 by resolving refs inside of `properties` and array `items` and restoring `100%` unit test coverage for `utils` and `validator-ajv8`
- Updated `package.json` to add `@types/jest` to the global `devDependencies`
- In `@rjsf/utils`, fixed rjsf-team#3671 and restored 100% test coverage as follows:
  - Updated `jest.config.js` to restore test coverage threshold to 100%
  - Updated `resolveAllReferences()` to take a new `recurseList: string[]` parameter that is used to prevent recurse `$ref` processing
    - Return the `schema` directly when it is not an object and also return the original `schema` when the `resolvedSchema` is identical
  - Updated the `resolveReference()` function to call `resolveAllReferences()` and only call `retrieveSchemaInternal()` when the the `updatedSchema` is different than the original
  - Updated the `resolveSchema()` function to always call `resolveReference()` and only return the `updatedSchemas` when something was changed
  - Updated `retrieveSchemaInternal()` to take a new `recurseList: string[] = []` parameter that is passed to `resolveSchema()` and `resolveCondition()`
    - Updated many of the internal functions to take a `recurseList: string[]` parameter that is forwarded to any function that ultimately calls `retrieveSchemaInternal()` or `resolveAllReferences()`
  - Updated the `SchemaParser` to properly pass the `recurseList` array to `retrieveSchemaInternal()` and `resolveAnyOrOneOfSchemas()`
  - Updated the `getClosestMatchingOption()` function to pass an empty array to the `resolveAllReferences()` function for `recurseList`
  - Updated the `computeDefaults()` function to pass an empty array to the `resolveDependencies()` function for `recurseList`
    - Also removed an unnecessary `isObject()` check for `formData` in when dealing with `additionalProperties` since it is always guaranteed to be an object
  - Added/updated tests to ensure that all of the `@rjsf/utils` have 100% test coverage
- In `@rjsf/validator-ajv8` to fix tests due to the rjsf-team#3671 changes and restored 100% test coverage as follows:
  - Updated `jest.config.js` to restore test coverage threshold to 100%
  - Updated the tests for `precompiledValidator` to call `retrieveSchema()` on the precompiled `rootSchema` to avoid errors caused by the `retrieveSchema()` changes
  - Updated `validator.ts` to make the `isValid()` call not check if the `rootSchema` already exists (because it doesn't) since the `finally` always removes it
    - Updated the `validator` tests to restore 100% test coverage
- Updated the `CHANGELOG.md` accordingly while also adding the description for rjsf-team#3870
heath-freenome added a commit to heath-freenome/react-jsonschema-form that referenced this issue Oct 6, 2023
…d validator-ajv8

Fixes rjsf-team#3671 by resolving refs inside of `properties` and array `items` and restoring `100%` unit test coverage for `utils` and `validator-ajv8`
- Updated `package.json` to add `@types/jest` to the global `devDependencies`
- In `@rjsf/utils`, fixed rjsf-team#3671 and restored 100% test coverage as follows:
  - Updated `jest.config.js` to restore test coverage threshold to 100%
  - Updated `resolveAllReferences()` to take a new `recurseList: string[]` parameter that is used to prevent recurse `$ref` processing
    - Return the `schema` directly when it is not an object and also return the original `schema` when the `resolvedSchema` is identical
  - Updated the `resolveReference()` function to call `resolveAllReferences()` and only call `retrieveSchemaInternal()` when the the `updatedSchema` is different than the original
  - Updated the `resolveSchema()` function to always call `resolveReference()` and only return the `updatedSchemas` when something was changed
  - Updated `retrieveSchemaInternal()` to take a new `recurseList: string[] = []` parameter that is passed to `resolveSchema()` and `resolveCondition()`
    - Updated many of the internal functions to take a `recurseList: string[]` parameter that is forwarded to any function that ultimately calls `retrieveSchemaInternal()` or `resolveAllReferences()`
  - Updated the `SchemaParser` to properly pass the `recurseList` array to `retrieveSchemaInternal()` and `resolveAnyOrOneOfSchemas()`
  - Updated the `getClosestMatchingOption()` function to pass an empty array to the `resolveAllReferences()` function for `recurseList`
  - Updated the `computeDefaults()` function to pass an empty array to the `resolveDependencies()` function for `recurseList`
    - Also removed an unnecessary `isObject()` check for `formData` in when dealing with `additionalProperties` since it is always guaranteed to be an object
  - Added/updated tests to ensure that all of the `@rjsf/utils` have 100% test coverage
- In `@rjsf/validator-ajv8` to fix tests due to the rjsf-team#3671 changes and restored 100% test coverage as follows:
  - Updated `jest.config.js` to restore test coverage threshold to 100%
  - Updated the tests for `precompiledValidator` to call `retrieveSchema()` on the precompiled `rootSchema` to avoid errors caused by the `retrieveSchema()` changes
  - Updated `validator.ts` to make the `isValid()` call not check if the `rootSchema` already exists (because it doesn't) since the `finally` always removes it
    - Updated the `validator` tests to restore 100% test coverage
- Updated the `CHANGELOG.md` accordingly while also adding the description for rjsf-team#3870
heath-freenome added a commit that referenced this issue Oct 10, 2023
…or-ajv8 (#3895)

Fixes #3671 by resolving refs inside of `properties` and array `items` and restoring `100%` unit test coverage for `utils` and `validator-ajv8`
- Updated `package.json` to add `@types/jest` to the global `devDependencies`
- In `@rjsf/utils`, fixed #3671 and restored 100% test coverage as follows:
  - Updated `jest.config.js` to restore test coverage threshold to 100%
  - Updated `resolveAllReferences()` to take a new `recurseList: string[]` parameter that is used to prevent recurse `$ref` processing
    - Return the `schema` directly when it is not an object and also return the original `schema` when the `resolvedSchema` is identical
  - Updated the `resolveReference()` function to call `resolveAllReferences()` and only call `retrieveSchemaInternal()` when the the `updatedSchema` is different than the original
  - Updated the `resolveSchema()` function to always call `resolveReference()` and only return the `updatedSchemas` when something was changed
  - Updated `retrieveSchemaInternal()` to take a new `recurseList: string[] = []` parameter that is passed to `resolveSchema()` and `resolveCondition()`
    - Updated many of the internal functions to take a `recurseList: string[]` parameter that is forwarded to any function that ultimately calls `retrieveSchemaInternal()` or `resolveAllReferences()`
  - Updated the `SchemaParser` to properly pass the `recurseList` array to `retrieveSchemaInternal()` and `resolveAnyOrOneOfSchemas()`
  - Updated the `getClosestMatchingOption()` function to pass an empty array to the `resolveAllReferences()` function for `recurseList`
  - Updated the `computeDefaults()` function to pass an empty array to the `resolveDependencies()` function for `recurseList`
    - Also removed an unnecessary `isObject()` check for `formData` in when dealing with `additionalProperties` since it is always guaranteed to be an object
  - Added/updated tests to ensure that all of the `@rjsf/utils` have 100% test coverage
- In `@rjsf/validator-ajv8` to fix tests due to the #3671 changes and restored 100% test coverage as follows:
  - Updated `jest.config.js` to restore test coverage threshold to 100%
  - Updated the tests for `precompiledValidator` to call `retrieveSchema()` on the precompiled `rootSchema` to avoid errors caused by the `retrieveSchema()` changes
  - Updated `validator.ts` to make the `isValid()` call not check if the `rootSchema` already exists (because it doesn't) since the `finally` always removes it
    - Updated the `validator` tests to restore 100% test coverage
- Updated the `CHANGELOG.md` accordingly while also adding the description for #3870
@BlackBerryID
Copy link

BlackBerryID commented Dec 4, 2023

@heath-freenome Hello. What about fields, that appears conditionally?
Example:
image
The field customerInformationSecondBlock resolves just with "$ref":
image

I made call this way: retrieveSchema(validator, jsonSchema, schemaWithDefinitions, formData);

@heath-freenome
Copy link
Member

@BlackBerryID can you create a new issue with a reproducible test case... Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug p1 Important to fix soon utils Related to @rjsf/utils
Projects
None yet
4 participants