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
fix: #3239 by providing an ErrorSchemaBuilder class in @rjsf/utils #3307
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, much cleaner
}); | ||
}); | ||
it("setting error string list without a path replaces at the root", () => { | ||
expect(builder.setErrors(SOME_ERRORS).ErrorSchema).toEqual({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am personally not a fan of relying on the order of the tests to validate behavior, and would rather we start from scratch in beforeEach
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the differences in behavior of the addErrors()
and setErrors()
it seemed easier to build the tests this way. Did you want me to change it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's fine, I agree this is easier and less code for something simple like this builder. Just a feeler to see how strongly you might agree 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general I'm ok with doing incremental testing like this when it makes sense. Your feeler has revealed I weakly agree with you :)
…/utils fix: rjsf-team#3239 by providing a new `ErrorSchemaBuilder` class in `@rjsf/utils` - In `@rjsf/utils` added `ErrorSchemaBuilder` to facilitate building `ErrorSchema` objects without the need for fancy casting - Exported the new class as part of the main `index.js` - Added 100% unit tests - In `@rjsf/validator-ajv6` and `@rjsf/validator-ajv8` updated the `toErrorSchema()` function to use the `ErrorSchemaBuilder` to simplify the implementation - Also updated the tests to use the `ErrorSchemaBuilder` to replace the expected values that required doing `as ErrorSchema` casting - Updated the `utility-functions.md` file to document `ErrorSchemaBuilder` - Updated the `CHANGELOG.md` accordingly for this fix as well as PR rjsf-team#3297
8f0ed9a
to
cd34993
Compare
…/utils (rjsf-team#3307) fix: rjsf-team#3239 by providing a new `ErrorSchemaBuilder` class in `@rjsf/utils` - In `@rjsf/utils` added `ErrorSchemaBuilder` to facilitate building `ErrorSchema` objects without the need for fancy casting - Exported the new class as part of the main `index.js` - Added 100% unit tests - In `@rjsf/validator-ajv6` and `@rjsf/validator-ajv8` updated the `toErrorSchema()` function to use the `ErrorSchemaBuilder` to simplify the implementation - Also updated the tests to use the `ErrorSchemaBuilder` to replace the expected values that required doing `as ErrorSchema` casting - Updated the `utility-functions.md` file to document `ErrorSchemaBuilder` - Updated the `CHANGELOG.md` accordingly for this fix as well as PR rjsf-team#3297
…/utils (rjsf-team#3307) fix: rjsf-team#3239 by providing a new `ErrorSchemaBuilder` class in `@rjsf/utils` - In `@rjsf/utils` added `ErrorSchemaBuilder` to facilitate building `ErrorSchema` objects without the need for fancy casting - Exported the new class as part of the main `index.js` - Added 100% unit tests - In `@rjsf/validator-ajv6` and `@rjsf/validator-ajv8` updated the `toErrorSchema()` function to use the `ErrorSchemaBuilder` to simplify the implementation - Also updated the tests to use the `ErrorSchemaBuilder` to replace the expected values that required doing `as ErrorSchema` casting - Updated the `utility-functions.md` file to document `ErrorSchemaBuilder` - Updated the `CHANGELOG.md` accordingly for this fix as well as PR rjsf-team#3297
Reasons for making this change
fix: #3239 by providing a new
ErrorSchemaBuilder
class in@rjsf/utils
@rjsf/utils
addedErrorSchemaBuilder
to facilitate buildingErrorSchema
objects without the need for fancy castingindex.js
@rjsf/validator-ajv6
and@rjsf/validator-ajv8
updated thetoErrorSchema()
function to use theErrorSchemaBuilder
to simplify the implementationErrorSchemaBuilder
to replace the expected values that required doingas ErrorSchema
castingutility-functions.md
file to documentErrorSchemaBuilder
CHANGELOG.md
accordingly for this fix as well as PR fix(material-ui): fix RangeWidget onChange handler #2161 #3297Checklist
npm run test:update
to update snapshots, if needed.