Bump/ajv #2542

newt10 · 2021-09-10T01:18:42Z

Reasons for making this change

AJV@6 is EOL. This PR updates the ajv to v7.

If this is related to existing tickets, include links to them as well. Use the syntax fixes #2541 .

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 updated docs if needed
I'm adding a new feature
- I've updated the playground with an example use of the feature

Tests

All tests have been updated to work with the new ajv library and all tests are passing

Playground

All playground forms are rendering as expected with the correct response submit.

epicfaace · 2021-09-10T21:53:11Z

docs/4.x upgrade guide.md

+
+### JSON Schema draft-04
+
+This version drops support for JSON schema draft04. 


Use "draft-04" and "draft-07"

epicfaace · 2021-09-10T21:53:43Z

docs/4.x upgrade guide.md

+
+## Breaking changes
+
+### JSON Schema draft-04


explicitly mention that we're bumping ajv to v7, therefore:

lost support for draft-04

etc.

epicfaace · 2021-09-10T22:02:53Z

packages/core/test/validate_test.js

+      // ajv@7.0^ has changed the format of error.dataPath. In ajv@6.0^ the error
+      // in the illformed key will result in dataPath = ['bar.\\'"[]()=+*&^%$#@!'].
+      // However in ajv@7.0 and higher this key results in
+      // dataPath = bar.'"[]()=+*&^%$#@!. lodash breaks this into error paths


note to @epicfaace -- look into why this is happening

@newt10 provide code sample to test this on ajv

@epicfaace Here is the code sample -> https://runkit.com/embed/qtzmcsm9k62w

Here's my own experiment: https://runkit.com/epicfaace/615235af9dcba10008c41a04

epicfaace

Let's also update https://github.com/rjsf-team/react-jsonschema-form/blob/3ec17f1c0ff40401b7a99c5e9891ac2834a1e73f/docs/usage/validation.md#custom-error-messages to reflect what we actually get (we might get a json pointer instead of a dot notation path)

epicfaace

JSON pointer spec: https://datatracker.ietf.org/doc/html/rfc6901

epicfaace

JSON pointer spec: https://datatracker.ietf.org/doc/html/rfc6901

epicfaace · 2021-09-27T21:50:37Z

packages/core/src/validate.js

+ * @param {string} property source of error in the data.
+ * @returns {stringp[]} array of object paths.
+ */
+function genPaths(property) {


Let's replace this with using the json-pointer npm library and use pointer.parse (https://www.npmjs.com/package/json-pointer)

epicfaace · 2021-09-27T21:55:20Z

packages/core/src/validate.js

-    validationError.message &&
-    typeof validationError.message === "string" &&
-    validationError.message.includes("no schema with key or ref ");
+  const includeValidationErrors = shouldIncludeSchemaErrors(validationError);


Comment: #1130 (comment)

Commit: 2e5d5b9

epicfaace · 2021-09-27T22:22:33Z

packages/core/src/validate.js

  } catch (err) {
    validationError = err;
  }

+  if (validationError === null) {
+    try {
+      ajv.validate(schema, formData);


ajv v7 changes:

If you call ajv.validate on an invalid schema twice, the second time it will (incorrectly) not throw any errors due to the schema being invalid.

In general, to note about ajv:

If you call ajv.validateSchema, it usually returns false if the schema is invalid. However, if the meta schema is not found, it will throw an exception.

Goal: Go around ajv v7 changes. Aim for same behavior as before (we want to catch all metaschema errors in validationError).

Current solution: Call ajv.validateSchema beforehand, but regardless of whether the schema is valid, call ajv.validate.

New solution: ???

kikonejacob · 2021-09-28T09:25:52Z

packages/core/src/validate.js

    allErrors: true,
    multipleOfPrecision: 8,
-    schemaId: "auto",
-    unknownFormats: "ignore",
+    strictTypes: "log",
  });



I suggest that we allow overriding ajv instance options.

Suggested change

function createAjvInstance(options) {

const ajv = new Ajv({

allErrors: true,

multipleOfPrecision: 8,

strictTypes: "log",

...options

});

That way we can pass the options in the form validation

export default function validateFormData( formData, schema, customValidate, transformErrors, additionalMetaSchemas = [], customFormats = {} validationOptions={} ) { // Include form data with undefined values, which is required for validation. const rootSchema = schema; formData = getDefaultFormState(schema, formData, rootSchema, true); const newMetaSchemas = !deepEquals(formerMetaSchema, additionalMetaSchemas); const newFormats = !deepEquals(formerCustomFormats, customFormats); if (newMetaSchemas || newFormats|| validationOptions) { ajv = createAjvInstance(validationOptions); }

This allows more control over validation without having to reimplement custom validate function
https://ajv.js.org/options.html

kikonejacob · 2021-10-01T19:27:22Z

@newt10 any update on this PR?

JulienBlacas · 2022-01-07T13:57:09Z

Is this PR still maintained by someone ? It would be definitely nice to have AJV v7 here :)

bollwyvl · 2022-02-05T21:06:46Z

as work has stalled here, and no work has happened on the ajv 7.x line, It seems like going straight to ajv 8 might make more sense at the next rjsf version bump. https://github.com/ajv-validator/ajv/blob/master/docs/v6-to-v8-migration.md

epicfaace · 2022-02-05T21:59:40Z

Agreed.

…

-- Ashwin Ramaswami

On Sat, Feb 5, 2022 at 4:06 PM Nicholas Bollweg ***@***.***> wrote: as work has stalled here, and no work has happened on the ajv 7.x line, It seems like going straight to ajv 8 might make more sense at the next rjsf version bump. https://github.com/ajv-validator/ajv/blob/master/docs/v6-to-v8-migration.md — Reply to this email directly, view it on GitHub <#2542 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAM4MX2Q6IKBFAAPN4TP6ODUZWGPDANCNFSM5DYJ2MZQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

bollwyvl · 2022-02-06T19:10:48Z

going straight to ajv 8 might make more sense at the next rjsf version bump

agreed

Another way to think about it, might be to have more of an interface-driven design, as with themes, that hid the ajv-specific exports.

This would potentially offer some trades between size, start-up time, and live performance, such as:

for backwards compatibility with draft4, a @rjsf/validator-ajv6
on-going effort targeted at improving a @rjsf/validator-ajv8
- a build could opt for a custom, pre-compiled validator
a la ajv dependency size #869, Build size too high #1625 one could use a lighter-weight validator... or a no-op validator

epicfaace · 2022-02-06T19:37:13Z

Another way to think about it, might be to have more of an interface-driven design, as with themes, that hid the ajv-specific exports.

That would indeed be nice!

bollwyvl · 2022-02-08T14:57:32Z

I've hoisted some of these ideas over here: #2693

heath-freenome · 2022-09-07T16:46:40Z

@newt10 Closing this as we have extracted the validation code into its own package(s) which is used as a required plug-in to Form. Also we are now providing an ajv8 based version of the validator see (@rjsf/validator-ajv8)

newt10 added ajv core labels Sep 10, 2021

newt10 self-assigned this Sep 10, 2021

newt10 linked an issue Sep 10, 2021 that may be closed by this pull request

Ajv6 is past EOL, upgrade AJV #2541

Closed

3 tasks

newt10 marked this pull request as ready for review September 10, 2021 01:35

epicfaace added the breaking change label Sep 10, 2021

epicfaace reviewed Sep 10, 2021

View reviewed changes

newt10 added 8 commits September 10, 2021 16:46

Install ajv@7 and ajv-formats

7fa846d

Update validate functions for ajv@7

5ca3c48

Update tests to check validate functions

5d6c538

Fix tests for minLength errors

24b6dd0

Fix tests with new error properties

92735cd

Fix tests with defs to $defs

74bc21c

Fix strict type warning for object tests

f6f4f3e

Documentation updates

2af6ae4

newt10 force-pushed the bump/ajv branch from 3931b19 to 2af6ae4 Compare September 11, 2021 00:15

epicfaace reviewed Sep 27, 2021

View reviewed changes

kikonejacob reviewed Sep 28, 2021

View reviewed changes

heath-freenome mentioned this pull request Jan 28, 2022

Add support for both Material UI 4 and 5 in @rjsf/material-ui (fix #2555) #2605

Merged

8 tasks

bollwyvl mentioned this pull request Feb 8, 2022

Decouple schema validation from core #2693

Closed

10 tasks

heath-freenome closed this Sep 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump/ajv #2542

Bump/ajv #2542

newt10 commented Sep 10, 2021 •

edited

Loading

epicfaace Sep 10, 2021

newt10 Sep 11, 2021

epicfaace Sep 10, 2021

newt10 Sep 11, 2021

epicfaace Sep 10, 2021

epicfaace Sep 10, 2021

newt10 Sep 10, 2021

epicfaace Sep 27, 2021

epicfaace left a comment

epicfaace left a comment

epicfaace left a comment

epicfaace Sep 27, 2021

epicfaace Sep 27, 2021

epicfaace Sep 27, 2021

kikonejacob Sep 28, 2021

kikonejacob Sep 28, 2021 •

edited

Loading

kikonejacob Sep 28, 2021

kikonejacob commented Oct 1, 2021

JulienBlacas commented Jan 7, 2022

bollwyvl commented Feb 5, 2022

epicfaace commented Feb 5, 2022 via email

bollwyvl commented Feb 6, 2022

epicfaace commented Feb 6, 2022

bollwyvl commented Feb 8, 2022

heath-freenome commented Sep 7, 2022


		### JSON Schema draft-04

		This version drops support for JSON schema draft04.

Bump/ajv #2542

Bump/ajv #2542

Conversation

newt10 commented Sep 10, 2021 • edited Loading

Reasons for making this change

Checklist

Tests

Playground

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

epicfaace left a comment

Choose a reason for hiding this comment

epicfaace left a comment

Choose a reason for hiding this comment

epicfaace left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kikonejacob Sep 28, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kikonejacob commented Oct 1, 2021

JulienBlacas commented Jan 7, 2022

bollwyvl commented Feb 5, 2022

epicfaace commented Feb 5, 2022 via email

bollwyvl commented Feb 6, 2022

epicfaace commented Feb 6, 2022

bollwyvl commented Feb 8, 2022

heath-freenome commented Sep 7, 2022

newt10 commented Sep 10, 2021 •

edited

Loading

kikonejacob Sep 28, 2021 •

edited

Loading