Fix ajv version to 5.0.0.beta-15

[testn]

Hey, I just made a Pull Request!

Upgrade ajv to version 5.0.0-beta15 and make sure that we use ajv8 rather than ajv6

✔️ Checklist

• A changeset describing the change and affected packages. (more info)
• Added or updated documentation
• Tests for new functionality and regression tests for bug fixes
• Screenshots attached (for UI changes)
• All your commits have a Signed-off-by line in the message. (more info)

[backstage-goalie]

Changed Packages

Package Name	Package Path	Changeset Bump	Current Version
@backstage/plugin-scaffolder	plugins/scaffolder	patch	v1.10.0-next.1

[backstage-goalie]

Thanks for the contribution!
All commits need to be DCO signed before they are reviewed. Please refer to the the DCO section in CONTRIBUTING.md or the DCO status for more info.

[github-actions]

Uffizzi Preview Environment deployment-9667

☁️ https://app.uffizzi.com/github.com/backstage/backstage/pull/15426

📄 View Application Logs etc.

What is Uffizzi? Learn more

[benjdlambert]

Hey @testn 👋 see #15321 for some explanation as to why we're going to stick using ajv6-validator for now, but we're happy to include the bump of the @rjsf packages.

[benjdlambert]

Ah wait - my bad - looks like they've fixed the issues. Perfect. rjsf-team/react-jsonschema-form#3309

@dagda1 do you wanna give this PR a twirl and see if it's working as expected now with the ajv8 validator?

[dagda1]

@benjdlambert If you look at the PR, the validation messages in the @rjsf PR point to the field name, which means I personally cannot use them or at least our clients won't accept that.

There is this old PR in @rjsf that would need to be upgraded to v5, which might be a plan to get better validation messages.

I might be missing something, but how can anyone have validation messages that point to the non-human readable field name?

I totally understand if you want to merge this PR.

[benjdlambert]

@dagda1 so maybe if we provide the option to also configure a validator, but default to the v8 one as it works for the Form use case as is, it's just that you need human readable names with the errorTransforms prop? or am I missing something?

[dagda1]

@dagda1 so maybe if we provide the option to also configure a validator, but default to the v8 one as it works for the Form use case as is, it's just that you need human readable names with the errorTransforms prop? or am I missing something?

@benjdlambert Here is the validation with ajv6:

Here is the validation with ajv8@5.0.0-beta.15

The inclusion of the fieldName makes this a no go for me but this is totally working correctly as far as ajv8 and @rjsf goes so I think you are right we should have some way to plug in different validators:

We could have a union string type added to the FormProps with the default being one of them?

  formProps: {
   validator: 'ajv6' | 'ajv8'

I could create a PR for this next week if we can agree on the approach now?

[benjdlambert]

@dagda1 I wonder though if you can use the transformErrors prop that's been added to FormProps to remove these field names? So you would just look for the regex of /must have required property '(.*)'/ and replace it with this field is required as the error or something?

[dagda1]

@dagda1 I wonder though if you can use the transformErrors prop that's been added to FormProps to remove these field names? So you would just look for the regex of /must have required property '(.*)'/ and replace it with this field is required as the error or something?

@benjdlambert Yes, that definitely works. Something like this

<NextScaffolderPage
  FormProps={{
    noHtml5Validate: true,
    transformErrors: errors =>
      errors.map(e =>
        typeof e.message === 'string' &&
        /^must have required property '(.*)'/.test(e.message)
          ? { ...e, message: 'is a required property' }
          : e,
      ),
  }}
/>

But are you saying anyone wanting this has to supply their own transformErrors function, or do we somehow make this available?

But publicly exposing transformErrors is now problematic with the latter approach.

Then again, I'm not too fond of the default experience of not offering a way to fix it OOTB.

[benjdlambert]

@dagda1 I wonder if we provide a default one that you can override still, but if you wanted to override it you would have to account for the required case too. Ideally I'd like to move off the ajv6 one as it's deprecated anyways, so the sooner we can get on to v8 with decent messages the better. I also wonder if this is kind of moot if we can get that other work done so that it will use the title instead of the key name and then it could be more human readable? Maybe we can look at porting that work to the v5 branch instead?

[dagda1]

@dagda1 I wonder if we provide a default one that you can override still, but if you wanted to override it you would have to account for the required case too. Ideally I'd like to move off the ajv6 one as it's deprecated anyways, so the sooner we can get on to v8 with decent messages the better. I also wonder if this is kind of moot if we can get that other work done so that it will use the title instead of the key name and then it could be more human readable? Maybe we can look at porting that work to the v5 branch instead?

@benjdlambert I agree ajv8 is the goal. I think we should do both. As the NextScaffolderPage is still alpha, we could provide a transformErrors as a short-term fix and then remove it when we get the other PR upgraded and merged into @rjsf/core. The tests are probably the most useful bit of that PR. It is a long way behind the current source. It is still all javascript in that PR.

[testn]

Do you think we can ask "ajv" to add an ability to customize error message?

https://ajv.js.org/packages/ajv-errors.html#messages-for-properties-and-items

[benjdlambert]

@dagda1 - feeling that rjsf-team/react-jsonschema-form#3337 is going to fix your issues right? So wondering if we should let this go ahead, given the fact that it will eventually be fixed upstream? Hopefully v5 will reach stable this week, so thinking once that happens we can start to roll out the next version for people to try officially, and see if there's any features that are missing from the current one and the next and look to deprecate the older one.

[dagda1]

@dagda1 - feeling that rjsf-team/react-jsonschema-form#3337 is going to fix your issues right? So wondering if we should let this go ahead, given the fact that it will eventually be fixed upstream? Hopefully v5 will reach stable this week, so thinking once that happens we can start to roll out the next version for people to try officially, and see if there's any features that are missing from the current one and the next and look to deprecate the older one.

There is still more to do on that PR if you look at this comment. I'm trying to see if I can get the extra work done another PR.

Would it be possible to wait until my PR is definitely merged before merging this? I don't want to make any assumptions about when my PR for @rjsf will be merged

But I totally understand if you want to merge it now

[benjdlambert]

@dagda1 na - it's fine to wait here until we've got some answers. All good.

[dagda1]

@benjdlambert @testn I got my PR merged. It has not been published yet, but it will either be 5.0.0-beta.17 or even a release 5.0.0.

[benjdlambert]

@dagda1 nice work on that! I pinged to get an update on when 5.0.0 is going to be released and hopefully should be within a week rjsf-team/react-jsonschema-form#3144 (comment) and I think once that is done we can replace this issue with the bump to v5.0.0 or even re-purpose this PR. That's kind of the last blocker before we can roll out the NextScaffolder for testing publicly. :)

[carl-reverb]

Perhaps related, but upon updating to Backstage 1.10.0 I see the following error in yarn dev when browser loads up:

Compiled with problems:X

WARNING in ../../node_modules/@rjsf/validator-ajv6/dist/validator-ajv6.esm.js 105:22-40

export 'ErrorSchemaBuilder' (imported as 'ErrorSchemaBuilder') was not found in '@rjsf/utils' (possible exports: ADDITIONAL_PROPERTIES_KEY, ADDITIONAL_PROPERTY_FLAG, ALL_OF_KEY, ANY_OF_KEY, CONST_KEY, DEFAULT_KEY, DEFINITIONS_KEY, DEPENDENCIES_KEY, ENUM_KEY, ERRORS_KEY, ID_KEY, ITEMS_KEY, NAME_KEY, ONE_OF_KEY, PROPERTIES_KEY, REF_KEY, REQUIRED_KEY, RJSF_ADDITONAL_PROPERTIES_FLAG, SUBMIT_BTN_OPTIONS_KEY, UI_FIELD_KEY, UI_OPTIONS_KEY, UI_WIDGET_KEY, allowAdditionalItems, asNumber, canExpand, createSchemaUtils, dataURItoBlob, deepEquals, findSchemaDefinition, getDefaultFormState, getDisplayLabel, getInputProps, getMatchingOption, getSchemaType, getSubmitButtonOptions, getTemplate, getUiOptions, getWidget, guessType, hasWidget, isConstant, isCustomWidget, isFilesArray, isFixedItems, isMultiSelect, isObject, isSelect, localToUTC, mergeDefaultsWithFormData, mergeObjects, mergeSchemas, mergeValidationData, optionsList, orderProperties, pad, parseDateString, processSelectValue, rangeSpec, retrieveSchema, schemaRequiresTrueValue, shouldRender, toConstant, toDateString, toIdSchema, toPathSchema, utcToLocal)

[benjdlambert]

@carl-reverb hmm that's unfortunate, could you maybe remove all occurences of @rjsf/* from your yarn.lock and reinstall? I wonder if this is the same runtime issue that you ran into @dagda1 with the #12378?

[dagda1]

Perhaps related, but upon updating to Backstage 1.10.0 I see the following error in yarn dev when browser loads up:

Compiled with problems:X

WARNING in ../../node_modules/@rjsf/validator-ajv6/dist/validator-ajv6.esm.js 105:22-40

export 'ErrorSchemaBuilder' (imported as 'ErrorSchemaBuilder') was not found in '@rjsf/utils' (possible exports: ADDITIONAL_PROPERTIES_KEY, ADDITIONAL_PROPERTY_FLAG, ALL_OF_KEY, ANY_OF_KEY, CONST_KEY, DEFAULT_KEY, DEFINITIONS_KEY, DEPENDENCIES_KEY, ENUM_KEY, ERRORS_KEY, ID_KEY, ITEMS_KEY, NAME_KEY, ONE_OF_KEY, PROPERTIES_KEY, REF_KEY, REQUIRED_KEY, RJSF_ADDITONAL_PROPERTIES_FLAG, SUBMIT_BTN_OPTIONS_KEY, UI_FIELD_KEY, UI_OPTIONS_KEY, UI_WIDGET_KEY, allowAdditionalItems, asNumber, canExpand, createSchemaUtils, dataURItoBlob, deepEquals, findSchemaDefinition, getDefaultFormState, getDisplayLabel, getInputProps, getMatchingOption, getSchemaType, getSubmitButtonOptions, getTemplate, getUiOptions, getWidget, guessType, hasWidget, isConstant, isCustomWidget, isFilesArray, isFixedItems, isMultiSelect, isObject, isSelect, localToUTC, mergeDefaultsWithFormData, mergeObjects, mergeSchemas, mergeValidationData, optionsList, orderProperties, pad, parseDateString, processSelectValue, rangeSpec, retrieveSchema, schemaRequiresTrueValue, shouldRender, toConstant, toDateString, toIdSchema, toPathSchema, utcToLocal)

@carl-reverb
I hit this too. I had to add the resolutions I mentioned here.

[carl-reverb]

THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.

Mmm, since upgrading backstage isn't an urgent matter, I'm not willing to resort to hacks like editing the lockfile or manipulating sub-sub-sub-package resolutions. I'll wait for 1.10.1 to be published.

[benjdlambert]

@carl-reverb happy to come up with a fix for a 1.10.1 release to try and make this easier, could you send me your yarn.lock so I can work out where the problem and incompatible libraries lie. I think this is an unfortunate side effect of moving some packages into another module that might cause hoisting of packages to work differently.

[benjdlambert]

Ok so I think from your comment @dagda1 I can see whats happened.

v1.9 we had the ajv-8 validator, which was would have have been installed and also the other dependencies which would all have been locked to a version, and then the upgrade would have caused only the ajv-6 validator to be installed again and re-evaluated so it pulled in a later version of the beta release which is incompatible with the older releases.

I think I can just bump all of them to the latest beta, and it should work as expected I think. Just confirming that now.

[testn]

Let's close this and rework the PR then

[benjdlambert]

@carl-reverb 1.10.1 has been released - can you give it a try to see if it works for you now and you don't get the runtime issue? @dagda1 you should also be able to remove the resolutions and give it a try with the latest version they shouldn't be needed.