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

Optimizations to improve performance on complex conditional schemas #2466

Closed
wants to merge 24 commits into from
Closed
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
eef551d
Implementation of If Then Else
Saulzi Feb 27, 2020
51387a0
Fix playground example
nickgros Jul 12, 2021
fca3592
Fix resolveSchema to handle more cases
nickgros Jul 12, 2021
ba15fd3
Don't remove the root schema after validating
nickgros Jul 12, 2021
75665ca
Warn when encountering error in validation
nickgros Jul 12, 2021
23288ae
Add tests to cover error cases
nickgros Jul 12, 2021
4cf5f7f
Merge branch 'master' of https://github.com/rjsf-team/react-jsonschem…
nickgros Jul 12, 2021
78fc62c
Memoize withIdRefPrefix and prevent recompiling during form validation
nickgros Jul 15, 2021
58d3335
Fix tests
nickgros Jul 15, 2021
5e7a08d
Memoize all the things
nickgros Jul 16, 2021
667903b
Import only memoize from lodash
nickgros Jul 22, 2021
ab36d1e
Revert outdated docs reorganization
nickgros Jul 22, 2021
b00f49f
Revert `const` changes
nickgros Jul 22, 2021
833006a
Remove default value from enum example
nickgros Jul 22, 2021
8796c9d
Delete `allOf` rather than set to undefined. Fixes liveOmit issue.
nickgros Jul 22, 2021
b6c53c8
Merge branch 'master' of https://github.com/rjsf-team/react-jsonschem…
nickgros Jul 22, 2021
f72a62c
Added tests
Juansasa Aug 6, 2021
75c4d84
Changed resolve method's name
Juansasa Aug 9, 2021
a08c563
Added $ref tests
Juansasa Aug 13, 2021
07a43c8
Merge branch 'master' into conditional-support
nickgros Sep 1, 2021
52747fb
Merge branch 'master' of https://github.com/rjsf-team/react-jsonschem…
nickgros Feb 10, 2022
123c480
Merge branch 'conditional-support' of github.com:nickgros/react-jsons…
nickgros Feb 10, 2022
964f537
Merge branch 'if_then_else' of https://github.com/stakater/react-json…
nickgros Feb 10, 2022
544133b
Merge branch 'master' of https://github.com/rjsf-team/react-jsonschem…
nickgros Feb 20, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 71 additions & 38 deletions packages/core/src/utils.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
import React from "react";
import * as ReactIs from "react-is";
import mergeAllOf from "json-schema-merge-allof";
import fill from "core-js-pure/features/array/fill";
import union from "lodash/union";
import jsonpointer from "jsonpointer";
import fields from "./components/fields";
import widgets from "./components/widgets";
import validateFormData, { isValid } from "./validate";
import _ from "lodash";
Copy link
Collaborator

Choose a reason for hiding this comment

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

To keep the build size down, it's preferable to import just the necessary function similarly to line 4.

Suggested change
import _ from "lodash";
import memoize from "lodash/memoize";


// Use the same default object to optimize memoized functions that rely on referential equality
const DEFAULT_ROOT_SCHEMA = {};
const DEFAULT_FORM_DATA = {};
export const ADDITIONAL_PROPERTY_FLAG = "__additional_property";

const widgetMap = {
Expand Down Expand Up @@ -171,11 +174,14 @@ export function hasWidget(schema, widget, registeredWidgets = {}) {
}
}

function computeDefaults(
const cacheKeyFn = (...args) => args.map(arg => JSON.stringify(arg)).join("_");
const computeDefaults = _.memoize(_computeDefaults, cacheKeyFn);

function _computeDefaults(
_schema,
parentDefaults,
rootSchema,
rawFormData = {},
rawFormData = DEFAULT_FORM_DATA,
includeUndefinedValues = false
) {
let schema = isObject(_schema) ? _schema : {};
Expand Down Expand Up @@ -303,7 +309,7 @@ function computeDefaults(
export function getDefaultFormState(
_schema,
formData,
rootSchema = {},
rootSchema = DEFAULT_ROOT_SCHEMA,
includeUndefinedValues = false
) {
if (!isObject(_schema)) {
Expand Down Expand Up @@ -521,7 +527,7 @@ export function toConstant(schema) {
}
}

export function isSelect(_schema, rootSchema = {}) {
export function isSelect(_schema, rootSchema = DEFAULT_ROOT_SCHEMA) {
const schema = retrieveSchema(_schema, rootSchema);
const altSchemas = schema.oneOf || schema.anyOf;
if (Array.isArray(schema.enum)) {
Expand All @@ -532,14 +538,18 @@ export function isSelect(_schema, rootSchema = {}) {
return false;
}

export function isMultiSelect(schema, rootSchema = {}) {
export function isMultiSelect(schema, rootSchema = DEFAULT_ROOT_SCHEMA) {
if (!schema.uniqueItems || !schema.items) {
return false;
}
return isSelect(schema.items, rootSchema);
}

export function isFilesArray(schema, uiSchema, rootSchema = {}) {
export function isFilesArray(
schema,
uiSchema,
rootSchema = DEFAULT_ROOT_SCHEMA
) {
if (uiSchema["ui:widget"] === "files") {
return true;
} else if (schema.items) {
Expand Down Expand Up @@ -584,7 +594,7 @@ export function optionsList(schema) {
}
}

export function findSchemaDefinition($ref, rootSchema = {}) {
export function findSchemaDefinition($ref, rootSchema = DEFAULT_ROOT_SCHEMA) {
const origRef = $ref;
if ($ref.startsWith("#")) {
// Decode URI fragment representation.
Expand Down Expand Up @@ -625,8 +635,8 @@ export const guessType = function guessType(value) {
// This function will create new "properties" items for each key in our formData
export function stubExistingAdditionalProperties(
schema,
rootSchema = {},
formData = {}
rootSchema = DEFAULT_ROOT_SCHEMA,
formData = DEFAULT_FORM_DATA
) {
// Clone the schema so we don't ruin the consumer's original
schema = {
Expand Down Expand Up @@ -662,21 +672,28 @@ export function stubExistingAdditionalProperties(
return schema;
}

export function resolveSchema(schema, rootSchema = {}, formData = {}) {
export const resolveSchema = _.memoize(_resolveSchema, cacheKeyFn);

export function _resolveSchema(
schema,
rootSchema = DEFAULT_ROOT_SCHEMA,
formData = DEFAULT_FORM_DATA
) {
if (schema.hasOwnProperty("$ref")) {
return resolveReference(schema, rootSchema, formData);
} else if (schema.hasOwnProperty("dependencies")) {
const resolvedSchema = resolveDependencies(schema, rootSchema, formData);
return retrieveSchema(resolvedSchema, rootSchema, formData);
} else if (schema.hasOwnProperty("allOf")) {
} else if (schema["allOf"]) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

what's the reason for this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

allOf may be set to undefined here: https://github.com/nickgros/react-jsonschema-form/blob/5e7a08dce80c0c1c85293ac3465c63d65693c6d4/packages/core/src/utils.js#L767

I think I changed that per a comment on the previous PR. Could use delete instead and revert this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switching that line to delete actually fixed the liveOmit issue that I mentioned here. I'm not sure if it fixed the stack overflow error that you saw.

return {
...schema,
allOf: schema.allOf.map(allOfSubschema =>
retrieveSchema(allOfSubschema, rootSchema, formData)
),
};
} else {
// No $ref or dependencies attribute found, returning the original schema.
// No $ref, dependencies, or allOf attribute found, so there's nothing to resolve.
// Returning the original schema.
return schema;
}
}
Expand All @@ -694,45 +711,40 @@ function resolveReference(schema, rootSchema, formData) {
);
}

export function retrieveSchema(schema, rootSchema = {}, formData = {}) {
export function _retrieveSchema(
schema,
rootSchema = DEFAULT_ROOT_SCHEMA,
formData = DEFAULT_FORM_DATA
) {
if (!isObject(schema)) {
return {};
return DEFAULT_ROOT_SCHEMA;
}
let resolvedSchema = resolveSchema(schema, rootSchema, formData);

const hasAdditionalProperties =
resolvedSchema.hasOwnProperty("additionalProperties") &&
resolvedSchema.additionalProperties !== false;

if (hasAdditionalProperties) {
resolvedSchema = stubExistingAdditionalProperties(
resolvedSchema,
rootSchema,
formData
);
}

if ("if" in resolvedSchema) {
while ("if" in resolvedSchema) {
// Note that if and else are key words in javascript so extract to variable names which are allowed
var {
if: expression,
then,
else: otherwise,
...resolvedSchemaLessConditional
} = resolvedSchema;
var conditionalSchema = isValid(expression, formData) ? then : otherwise;

var conditionalSchema = isValid(expression, formData, rootSchema)
? then
: otherwise;

if (conditionalSchema) {
conditionalSchema = resolveSchema(
conditionalSchema,
rootSchema,
formData
);
resolvedSchema = mergeSchemas(
resolvedSchemaLessConditional,
conditionalSchema
);
}
resolvedSchema = mergeSchemas(
resolvedSchemaLessConditional,
conditionalSchema || {}
);
}

let allOf = resolvedSchema.allOf;
Expand All @@ -743,22 +755,38 @@ export function retrieveSchema(schema, rootSchema = {}, formData = {}) {

// if we see an if in our all of schema then evaluate the if schema and select the then / else, not sure if we should still merge without our if then else
if ("if" in allOfSchema) {
allOfSchema = isValid(allOfSchema.if, formData)
allOfSchema = isValid(allOfSchema.if, formData, rootSchema)
? allOfSchema.then
: allOfSchema.else;
}

if (allOfSchema) {
allOfSchema = resolveSchema(allOfSchema, rootSchema, formData); // resolve references etc.
resolvedSchema = mergeSchemas(resolvedSchema, allOfSchema);
delete resolvedSchema.allOf;
resolvedSchema = {
...mergeSchemas(resolvedSchema, allOfSchema),
allOf: undefined,
};
}
}
}

const hasAdditionalProperties =
resolvedSchema.hasOwnProperty("additionalProperties") &&
resolvedSchema.additionalProperties !== false;

if (hasAdditionalProperties) {
resolvedSchema = stubExistingAdditionalProperties(
resolvedSchema,
rootSchema,
formData
);
}

return resolvedSchema;
}

export const retrieveSchema = _.memoize(_retrieveSchema, cacheKeyFn);

function resolveDependencies(schema, rootSchema, formData) {
// Drop the dependencies from the source schema.
let { dependencies = {}, ...resolvedSchema } = schema;
Expand Down Expand Up @@ -1029,7 +1057,7 @@ export function toIdSchema(
schema,
id,
rootSchema,
formData = {},
formData = DEFAULT_FORM_DATA,
idPrefix = "root"
) {
const idSchema = {
Expand Down Expand Up @@ -1061,7 +1089,12 @@ export function toIdSchema(
return idSchema;
}

export function toPathSchema(schema, name = "", rootSchema, formData = {}) {
export function toPathSchema(
schema,
name = "",
rootSchema,
formData = DEFAULT_FORM_DATA
) {
const pathSchema = {
$name: name.replace(/^\./, ""),
};
Expand Down
Loading