Skip to content

Commit abbecc5

Browse files
- Responded to /code-review skill
1 parent 6c33279 commit abbecc5

3 files changed

Lines changed: 76 additions & 6 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,18 @@ should change the heading of the (upcoming) version to include a major version b
3131
## @rjsf/core
3232

3333
- Support locale-specific decimal separators in `NumberField` (e.g. Polish/German comma decimal separators) without breaking non-text widgets like radio buttons and select dropdowns, fixing [#5148](https://github.com/rjsf-team/react-jsonschema-form/pull/5148)
34-
- Fixed `ObjectField` to preserve the insertion order of additional and pattern properties when keys are renamed to integers, fixing [#4631](https://github.com/rjsf-team/react-jsonschema-form/issues/4631) ([#5156](https://github.com/rjsf-team/react-jsonschema-form/pull/5156))
34+
- Fixed `ObjectField` to preserve the insertion order of additional and pattern properties when keys are renamed to integers, fixing [#4631](https://github.com/rjsf-team/react-jsonschema-form/issues/4631)
3535

3636
## @rjsf/utils
3737

3838
- Added `getDecimalSeparator()` utility to detect locale-specific decimal separators and updated `asNumber()` to parse locale-specific decimal strings, partially fixing [#5148](https://github.com/rjsf-team/react-jsonschema-form/pull/5148)
3939
- Fixed a regression where `getDefaultFormState()` with `experimental_defaultFormStateBehavior.arrayMinItems.populate = 'never'` padded arrays with `null` entries up to `minItems`, fixing [#5163](https://github.com/rjsf-team/react-jsonschema-form/issues/5163)
4040
- Optional arrays with no data are now omitted (unchanged) while required arrays with no data default to `[]`, letting the validator report the `minItems` violation instead of surfacing invalid `null` items
4141

42+
## @rjsf/validator-ajv8
43+
44+
- Updated `AJV8Validator` to remove broken schemas from AJV cache after compilation error, fixing [#3933](https://github.com/rjsf-team/react-jsonschema-form/issues/3933)
45+
4246
## @rjsf/validator-ata
4347

4448
- Updated the `ata-validator` dependency to `^1.1.0`, which fixes a stack overflow in browser environments for schemas its JS engine cannot compile and shrinks the install footprint via per-platform optional packages ([#5162](https://github.com/rjsf-team/react-jsonschema-form/pull/5162))

packages/validator-ajv8/src/validator.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,11 @@ export default class AJV8Validator<
114114
// the entry so every call with an invalid schema consistently produces a
115115
// compilation error. For schemas with $id the cache key is the id string;
116116
// for anonymous schemas AJV uses the object reference as the key.
117-
this.ajv.removeSchema(schema[ID_KEY] !== undefined ? schema[ID_KEY] : (schema as object));
117+
// Guard with compiledValidator === undefined so that a runtime error thrown
118+
// by compiledValidator(formData) does not evict a correctly-compiled schema.
119+
if (compiledValidator === undefined) {
120+
this.ajv.removeSchema(schema[ID_KEY] !== undefined ? schema[ID_KEY] : (schema as object));
121+
}
118122
}
119123

120124
let errors;
@@ -235,9 +239,10 @@ export default class AJV8Validator<
235239
* @param rootSchema - The root schema used to provide $ref resolutions
236240
*/
237241
isValid(schema: S, formData: T | undefined, rootSchema: S) {
238-
// schemaId is declared outside the try so the catch block can remove the
239-
// broken schema from AJV's registry even when an error occurs during compilation.
242+
// schemaId and compiled are declared outside the try so the catch block can
243+
// conditionally remove the broken schema from AJV's registry.
240244
let schemaId: string | undefined;
245+
let compiled = false;
241246
try {
242247
this.handleSchemaUpdate(rootSchema);
243248
// then rewrite the schema ref's to point to the rootSchema
@@ -255,15 +260,17 @@ export default class AJV8Validator<
255260
this.ajv.addSchema(schemaWithIdRefPrefix, schemaId).getSchema(schemaId) ||
256261
this.ajv.compile(schemaWithIdRefPrefix);
257262
}
263+
compiled = true;
258264
const result = compiledValidator(formData);
259265
return result;
260266
} catch (e) {
261267
// oxlint-disable-next-line no-console
262268
console.warn('Error encountered compiling schema:', e);
263269
// Remove the broken schema from AJV's registry so a subsequent rawValidation
264270
// or isValid call does not silently reuse a cached entry that bypassed
265-
// meta-schema validation.
266-
if (schemaId !== undefined) {
271+
// meta-schema validation. Guard with !compiled so that a runtime error thrown
272+
// by compiledValidator(formData) does not evict a correctly-compiled schema.
273+
if (!compiled && schemaId !== undefined) {
267274
this.ajv.removeSchema(schemaId);
268275
}
269276
return false;

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

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import type {
77
ValidatorType,
88
} from '@rjsf/utils';
99
import { ErrorSchemaBuilder } from '@rjsf/utils';
10+
import type Ajv from 'ajv';
1011
import localize from 'ajv-i18n';
1112
import Ajv2019 from 'ajv/dist/2019';
1213
import Ajv2020 from 'ajv/dist/2020';
@@ -258,6 +259,64 @@ describe('AJV8Validator', () => {
258259
expect(result1).toBe(false);
259260
expect(result2).toBe(false);
260261
});
262+
263+
it('rawValidation does not evict a correctly-compiled schema when data evaluation throws', () => {
264+
// A keyword whose validate() throws during data evaluation (not compilation).
265+
// The schema is valid and compiles fine; the throw happens when the compiled
266+
// validator runs against formData. The fix guards removeSchema with
267+
// compiledValidator === undefined so only compile-phase failures evict.
268+
const v = new AJV8Validator({
269+
extenderFn: (ajv: Ajv) => {
270+
ajv.addKeyword({
271+
keyword: 'throwOnValidate',
272+
schemaType: 'boolean',
273+
validate: () => {
274+
throw new Error('keyword threw during data evaluation');
275+
},
276+
});
277+
return ajv;
278+
},
279+
});
280+
const schema = { $id: 'test-exec-throw-raw', type: 'string', throwOnValidate: true } as unknown as RJSFSchema;
281+
282+
// First call — compiledValidator(formData) throws, so compilationError is set.
283+
const result = v.rawValidation(schema, 'hello');
284+
expect(result.validationError).toBeInstanceOf(Error);
285+
286+
// Schema must still be cached — the execution throw must not have evicted it.
287+
expect(v.ajv.getSchema('test-exec-throw-raw')).toBeDefined();
288+
});
289+
290+
it('isValid does not evict a correctly-compiled schema when data evaluation throws', () => {
291+
const v = new AJV8Validator({
292+
extenderFn: (ajv: Ajv) => {
293+
ajv.addKeyword({
294+
keyword: 'throwOnValidate',
295+
schemaType: 'boolean',
296+
validate: () => {
297+
throw new Error('keyword threw during data evaluation');
298+
},
299+
});
300+
return ajv;
301+
},
302+
});
303+
const schema = {
304+
$id: 'test-exec-throw-isvalid',
305+
type: 'string',
306+
throwOnValidate: true,
307+
} as unknown as RJSFSchema;
308+
const rootSchema: RJSFSchema = {};
309+
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(noop);
310+
311+
// Execution throw — isValid should return false but NOT evict the schema.
312+
const result = v.isValid(schema, 'hello', rootSchema);
313+
expect(result).toBe(false);
314+
315+
// $id is preserved by withIdRefPrefix, so schemaId === schema.$id.
316+
expect(v.ajv.getSchema('test-exec-throw-isvalid')).toBeDefined();
317+
318+
warnSpy.mockRestore();
319+
});
261320
});
262321
describe('validator.validateFormData()', () => {
263322
describe('No custom validate function, single value', () => {

0 commit comments

Comments
 (0)