fix: Implement precompiled validator support in @rjsf/validator-ajv8 #3652

heath-freenome · 2023-05-09T23:55:46Z

Reasons for making this change

Fixes #3543 by implementing support for precompiled validators

In @rjsf/validator-ajv8 added support for precompiled validators as follows:
- Added a new compileSchemaValidators() API function used to generate the precompiled validators for a schema to an output file
- Updated the documentation for the customizeValidator() API function
- Added a new AJV8PrecompiledValidator implementation of the ValidatorType interface
- Refactored a large piece of the raw validation error processing from the AJV8Validator into a new processRawValidationErrors() function also used by the AJV8PrecompiledValidator
- Added a new usePrecompiledValidator() API function that is similar to customizeValidator() but returning a precompiled validator-based ValidatorType interface implementation
- Added some new types to the types.ts file in support of precompiled validators
- Updated the main index.ts file to export the new types and API functions
- Added 100% unit test coverage of the new feature
  - This included implementing a node function to precompile the superSchema.json file found in the test/harness directory
Updated the validation.md documentation for the new precompiled validator functionality
Added a new validator-ajv8.md documentation file to the api-reference directory and the sidebar.js
Updated the CHANGELOG.md file accordingly

Checklist

I'm updating documentation
- I've checked the rendering of the Markdown text I've added
I'm adding or updating code
- I've added and/or updated tests. I've run npm run test:update to update snapshots, if needed.
- I've updated docs if needed
- I've updated the changelog with a description of the PR
I'm adding a new feature
- I've updated the playground with an example use of the feature

packages/validator-ajv8/src/compileSchemaValidators.ts

packages/validator-ajv8/src/precompiledValidator.ts

packages/validator-ajv8/test/precompiledValidator.test.ts

Fixes rjsf-team#3543 by implementing support for precompiled validators - In `@rjsf/validator-ajv8` added support for precompiled validators as follows: - Added a new `compileSchemaValidators()` API function used to generate the precompiled validators for a schema to an output file - Updated the documentation for the `customizeValidator()` API function - Added a new `AJV8PrecompiledValidator` implementation of the `ValidatorType` interface - Refactored a large piece of the raw validation error processing from the `AJV8Validator` into a new `processRawValidationErrors()` function also used by the `AJV8PrecompiledValidator` - Added a new `usePrecompiledValidator()` API function that is similar to `customizeValidator()` but returning a precompiled validator-based `ValidatorType` interface implementation - Added some new types to the `types.ts` file in support of precompiled validators - Updated the main `index.ts` file to export the new types and API functions - Added 100% unit test coverage of the new feature - This included implementing a node function to precompile the `superSchema.json` file found in the `test/harness` directory - Added `ignorePatterns` to the `.eslintrc` file to ignore the precompiled schema files - Updated the `validation.md` documentation for the new precompiled validator functionality - Added a new `validator-ajv8.md` documentation file to the `api-reference` directory and the `sidebar.js` - Updated the `CHANGELOG.md` file accordingly

…n the file

… improved the `CHANGELOG.md` comments related to rjsf-team#3546

nickgros · 2023-05-11T00:06:34Z

Just leaving a note to cross-reference with #1961 and follow-up later...while this isn't a silver bullet for that issue, I'm curious to see how much better the pre-compiled validator performs when using schemas with high conditional complexity.

nickgros · 2023-05-11T00:12:14Z

packages/validator-ajv8/src/precompiledValidator.ts

+   * @returns - The precompiled validator function associated with this schema
+   */
+  getValidator(schema: S) {
+    const key = get(schema, ID_KEY, hashForSchema(schema));


We shouldn't need to compute the hash if we have an $id. This is not a big deal if the hash is not an expensive computation.

nickgros · 2023-05-11T00:16:26Z

packages/docs/docs/api-reference/validator-ajv8.md

+- output: string - The name of the file into which the precompiled validator functions will be generated
+- [options={}]: CustomValidatorOptionsType - The set of `CustomValidatorOptionsType` information used to alter the AJV validator used for compiling the schema. They are the same options that are passed to the `customizeValidator()` function in order to modify the behavior of the regular AJV-based validator.
+
+### usePrecompiledValidator<T = any, S extends StrictRJSFSchema = RJSFSchema, F extends FormContextType = any>()


use... makes this look like a React hook, so I would suggest a slightly different name, like

Suggested change

### usePrecompiledValidator<T = any, S extends StrictRJSFSchema = RJSFSchema, F extends FormContextType = any>()

### createPrecompiledValidator<T = any, S extends StrictRJSFSchema = RJSFSchema, F extends FormContextType = any>()

nickgros · 2023-05-11T00:25:59Z

packages/validator-ajv8/src/types.ts

+export interface CompiledValidateFunction<T = any> extends Omit<ValidateFunction<T>, 'schema' | 'schemaEnv'> {
+  /** This is literally copied from the `ValidateFunction` type definition from which it extends because it seems to get
+   * lost as part of the Omit<>.
+   */
+  (this: Ajv | any, data: any, dataCxt?: DataValidationCxt): boolean;
+}


Tbh I don't really know how a function definition is supposed to work with Omit, but it makes sense to do this, since it also has properties you want to drop.

nickgros · 2023-05-11T00:27:10Z

packages/validator-ajv8/src/usePrecompiledValidator.ts

+ * @param [localizer] - If provided, is used to localize a list of Ajv `ErrorObject`s
+ * @returns - The precompiled validator implementation resulting from the set of parameters provided
+ */
+export default function usePrecompiledValidator<


As mentioned before, prefer a different name so this doesn't look like a React hook.

nickgros · 2023-05-11T00:31:06Z

packages/validator-ajv8/test/harness/compileTestSchema.js

+ * In order to keep things in sync, it may be necessary to run this after making changes in the schemaParser world in
+ * `@rjsf/utils` OR if an AJV update is installed. To run this, simply do the following, starting in the root directory
+ * of the `@rjsf/validator-ajv8` directory:
+ *
+ * - cd test/harness
+ * - node compileTestSchema.js
+ *
+ * Then add the two updated `superSchema.js` and `superSchemaOptions.js` files to your PR


Hmm...we'd have no idea if something breaks unless we're disciplined about following these steps. Is it burdensome to just run this file and recompile the validators before we run the tests?

And if so, we can .gitignore the compiled files, of course

Hmmm, good idea, let me see how that goes

nickgros · 2023-05-11T00:35:05Z

packages/validator-ajv8/test/harness/compileTestSchema.js

+const compileSchemaValidators = require('../../dist').compileSchemaValidators;
+const superSchema = require('./superSchema.json');
+
+// NOTE these are the same as the CUSTOM_OPTIONS in `testData.ts`, keep them in sync


Why can't we just import this from testData?

The TS file is using imports for types which don't work in a CJS world that this file is running in.

Gotcha, so this is easier than using ts-node or something like that. That's fine.

nickgros · 2023-05-11T00:42:13Z

packages/validator-ajv8/test/precompiledValidator.test.ts

Fantastic set of tests

nickgros · 2023-05-11T00:44:29Z

packages/validator-ajv8/test/usePrecompiledValidator.test.ts

+  describe('passing validatorFns and rootSchema to usePrecompiledValidator', () => {
+    let custom: any;
+    beforeAll(() => {
+      (AJV8PrecompiledValidator as unknown as jest.Mock).mockClear();


You can probably use jest.mocked() instead of this sequence of casts - https://jestjs.io/docs/mock-function-api/#jestmockedsource-options

Wow, I just learned something new

I just learned about it recently 🙂

nickgros · 2023-05-11T00:48:20Z

packages/validator-ajv8/test/validator.test.ts

@@ -32,10 +25,10 @@ describe('AJV8Validator', () => {
    builder.resetAllErrors();
  });
  describe('default options', () => {
-    // Use the TestValidator to access the `withIdRefPrefix` function
-    let validator: TestValidator;
+    // Use the AJV8Validator to access the `withIdRefPrefix` function


Does it make sense to remove this comment (and the other 3 times it appears in this file)? Seems like the refactor made this easy to clean up and we're not even accessing withIdRefPrefix in this file

Suggested change

// Use the AJV8Validator to access the `withIdRefPrefix` function

Good catch!

nickgros

Looks great! Excited to see this released!

heath-freenome requested review from epicfaace, jacqueswho and nickgros May 9, 2023 23:55

heath-freenome force-pushed the fix-3543-by-implementing-precompiled-validators branch from 4f94b29 to f80ed1b Compare May 10, 2023 02:46

heath-freenome mentioned this pull request May 10, 2023

fix: set defaults for oneOf + ref #3634

Merged

8 tasks

heath-freenome force-pushed the fix-3543-by-implementing-precompiled-validators branch from f80ed1b to eb7a423 Compare May 10, 2023 14:53