Replace json-schema-merge-allof with @x0k/json-schema-merge #4881
Conversation
| "noFallthroughCasesInSwitch": true, | ||
| "allowSyntheticDefaultImports": true, | ||
| "moduleResolution": "node", | ||
| "moduleResolution": "bundler", |
There was a problem hiding this comment.
I could not import from @x0k/json-schema-merge/lib/array without changing this. I think the old node resolution did not respect the package exports
There was a problem hiding this comment.
Yeah, I'm actually surprised I didn't change that one along with the rest that I did change
| - Updated test files to switch to using the React Testing library instead of `react-test-renderer`, updating all snapshots accordingly | ||
| - Updated Fluent libraries to latest, with updated snapshots | ||
| - Added Node 24 to `ci.yml`, removing `v5` and old Node 14 non-docs builds | ||
| - Added Node 24 to `ci.yml`, removing `v5` and old Node 14 non-docs builds |
There was a problem hiding this comment.
This and below was automatic whitespace formatting
| // With Lerna active, the test world has access to the test suite via the symlink | ||
| import { TestValidatorType } from '@rjsf/utils/test/schema'; |
There was a problem hiding this comment.
This stopped working after I updated the TypeScript configuration, likely because ./test/schema is not actually exported by @rjsf/utils.
Surprisingly, a relative path to the utils folder didn't break anything, so let's go with that.
| food: { type: 'string', enum: ['meat', 'grass', 'fish'] }, | ||
| }, | ||
| required: ['animal', 'food'], | ||
| required: ['food', 'animal'], |
There was a problem hiding this comment.
The order of these changed, but otherwise all tests passed
| deep: false, | ||
| resolvers: { | ||
| $defs: mergeAllOf.options.resolvers.definitions, | ||
| }, |
There was a problem hiding this comment.
One thing I am unsure of is if the behavior this option controlled is supported by @x0k/json-schema-merge. Since resolution of references is a non-goal of that project, I would expect not.
However, we don't have a test for this, so I'm not sure if we were relying on json-schema-merge-allof to do resolution in the first place, and I don't know exactly what kinds schemas this might affect.
There was a problem hiding this comment.
Have you tried asking the author of the library whether it does this behavior, and if so, how to configure it?
There was a problem hiding this comment.
deep: false->createShallowAllOfMerge- Merging
$defsis supported source
| deep: false, | ||
| resolvers: { | ||
| $defs: mergeAllOf.options.resolvers.definitions, | ||
| }, |
There was a problem hiding this comment.
Have you tried asking the author of the library whether it does this behavior, and if so, how to configure it?
| "noFallthroughCasesInSwitch": true, | ||
| "allowSyntheticDefaultImports": true, | ||
| "moduleResolution": "node", | ||
| "moduleResolution": "bundler", |
There was a problem hiding this comment.
Yeah, I'm actually surprised I didn't change that one along with the rest that I did change
- additional feedback
3 participants
Reasons for making this change
Resolves #4774
json-schema-merge-allofwith@x0k/json-schema-mergeexportsfield for importsjson-schema-merge-allofChecklist
npx nx run-many --target=build --exclude=@rjsf/docs && npm run test:updateto update snapshots, if needed.