Skip to content
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

added limited support for conditional property values #135

Merged
merged 2 commits into from
Sep 6, 2024

Conversation

imolorhe
Copy link
Collaborator

@imolorhe imolorhe commented Jul 2, 2024

As mentioned in #129 we don't get fully resolved schemas based on the data. Following the discussion in sagold/json-schema-library#61, that appears to be expected.

The workaround I implemented is to "fake" a deep schema resolution by performing the shallow schema resolution on the direct children properties of the schema (we only really care about the direct children in this context).

@imolorhe imolorhe requested a review from acao July 2, 2024 17:27
Copy link

changeset-bot bot commented Jul 2, 2024

🦋 Changeset detected

Latest commit: fee04ce

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
codemirror-json-schema Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

netlify bot commented Jul 2, 2024

Deploy Preview for codemirror-json-schema ready!

Name Link
🔨 Latest commit fee04ce
🔍 Latest deploy log https://app.netlify.com/sites/codemirror-json-schema/deploys/6684389df13a5d0008eae164
😎 Deploy Preview https://deploy-preview-135--codemirror-json-schema.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@imolorhe imolorhe marked this pull request as ready for review July 2, 2024 17:28
Copy link
Collaborator

@acao acao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great! we will find a more comprehensive solution for inferring data types soon, hopefully we can contribute back where it makes sense. this looks pretty solid for now

src/features/completion.ts Show resolved Hide resolved
@acao acao merged commit 917333e into main Sep 6, 2024
6 checks passed
@acao acao deleted the imolorhe/conditional-prop-values branch September 6, 2024 01:56
acao added a commit that referenced this pull request Sep 6, 2024
acao added a commit that referenced this pull request Sep 6, 2024
Reverts #135, I did not see the
alternative PR sorry!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants