Much more robust auto-completion suggestions for conditional types (even when current data is invalid) #138
Conversation
|
✅ Deploy Preview for codemirror-json-schema ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @thomasjahoda thank you for this! I'm currently looking into the weird vitest failure. can you add a changset as well? |
| const schemaValue = localStorage.getItem("selectedSchema")!; | ||
| | ||
| const setFileName = (value) => { | ||
| const setFileName = (value: any) => { |
There was a problem hiding this comment.
not a huge deal because it's just the demo, but wouldn't this be string?
| ], | ||
| test: { | ||
| maxConcurrency: 10, | ||
| // configuration to be able to view console.log messages while debugging |
There was a problem hiding this comment.
I think we may need to disable or toggle something here in Github Actions
| * removes required properties and allows additional properties everywhere | ||
| * @param schema | ||
| */ | ||
| function makeSchemaLax(schema: any): any { |
src/features/completion.ts Outdated
| | ||
| // TODO also resolve patternProperties of allOf, anyOf, oneOf | ||
| return { | ||
| ...omit(subSchema, ["allOf", "anyOf", "oneOf"]), |
There was a problem hiding this comment.
there is an easier way to do this that doesn't require an additional dependency. radash looks cool but I don't think we need it right now, what do you think @imolorhe ?
an ecmascript native approach would be something more like this:
const { allOf, anyOf, oneOf, ...simpleSchema } = subSchema return { ...simpleSchema, properties: effectiveProperties, };
There was a problem hiding this comment.
Yeah we shouldn't be adding that extra dependency.
| @thomasjahoda i merged your other PR first so this needs a rebase. just let me know when you are able to remove radash for now and we're good to go! |
| @thomasjahoda are you interested in continuing this PR? |
| heads up @imolorhe I plan on re-creating this PR and making the one change we need, if we don't hear back from @thomasjahoda soon |
| @acao Sorry that I did not react to my inbox at all. Sadly, I don't think that I should spend more time on this in the near future and I would be very glad if someone else continues this. Please feel free to use/change my code/PR however you like. |
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## codemirror-json-schema@0.8.0 ### Minor Changes - [#138](#138) [`aa27ad7`](aa27ad7) Thanks [@thomasjahoda](https://github.com/thomasjahoda)! - More robust conditional types support (thanks @thomasjahoda!) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
3 participants
I continued the work on properly supporting conditional types. Conditional types were first supported via #133, quite recently.
A lot of cases were not working at all, specifically whenever the data was not valid for the current schema and whenever a new property was being added in a dynamic schema, where the parent does not have enough info for the types.
I added some tests for the scenarios I needed to support. And after verifying the old tests are working, I added
additionalProperties: falseto some existing test-data schemas, as that complicates matters.@imolorhe I didn't see your work in #135 (branch
conditional-prop-values), only the commits already merged into master due to #133. I chose the same approach as you to workaround the limitations of json-schema-library: to get the list of all properties in the current object, I just ask json-schema-library to resolve the JSON pointers for all possible direct properties I can find.