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

fix: does not fetch json-schema-07 from web. #508

Merged
merged 5 commits into from
Dec 19, 2022

Conversation

magicmatatjahu
Copy link
Member

@magicmatatjahu magicmatatjahu commented Nov 25, 2022

Description

  • does not fetch json-schema-07 from web - serialize the specification to have proper uri for json-schema-draft-07 JSON Schema.

TESTING PR.

Related issue
Fixes #509

@magicmatatjahu magicmatatjahu added the bug Something isn't working label Nov 25, 2022
@netlify
Copy link

netlify bot commented Nov 25, 2022

Deploy Preview for modest-rosalind-098b67 ready!

Name Link
🔨 Latest commit c299fda
🔍 Latest deploy log https://app.netlify.com/sites/modest-rosalind-098b67/deploys/63a02d44f41529000843b456
😎 Deploy Preview https://deploy-preview-508--modest-rosalind-098b67.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 settings.

@fmvilas
Copy link
Member

fmvilas commented Dec 12, 2022

I'm surprised how much you had to do to fix this 🤔 Isn't there a simpler way? We shouldn't be the only ones having this problem.

@magicmatatjahu
Copy link
Member Author

magicmatatjahu commented Dec 12, 2022

@fmvilas I tried using the http protocol but it didn't work, then I tried removing the schema itself from our definitions and it still didn't work. Only after entering the schema as https (we have to do this manually and change every reference to https) does it work. Both validation and autocompletion. If you have another (worked) idea, you're welcome with contribution 😄

@fmvilas
Copy link
Member

fmvilas commented Dec 12, 2022

So changing the http to https here didn't work?

@magicmatatjahu
Copy link
Member Author

@fmvilas uri ($id field) shouldn't point to the https protocol because json-schema-draft-07 exist as http://json-schema.org/draft-07/schema not https://json-schema.org/draft-07/schema - we shouldn't change $id of default json schema definition. We bundle http://json-schema.org/draft-07/schema just to make it easier for other people, but in the case of the yaml plugin for monaco (in the Studio case) it doesn't work because it only works for https if origin is https and http(s) if origin is http.

@fmvilas
Copy link
Member

fmvilas commented Dec 12, 2022

Oof! 🤦 That's bad. I thought that JSON Schema was serving https://json-schema.org/draft-07/schema with the "$id": "https://json-schema.org/draft-07/schema" value. Would have been great if they could detect when HTTPS or HTTP is used and update accordingly. I know the URL shouldn't strictly be $id (and vice versa) but it would be very handy.

Pinging @Relequestual.

Copy link
Member

@fmvilas fmvilas left a comment

Choose a reason for hiding this comment

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

Approving as this solution is better than what we have right now.

@sonarcloud
Copy link

sonarcloud bot commented Dec 19, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@magicmatatjahu
Copy link
Member Author

/rtm

@asyncapi-bot asyncapi-bot merged commit 5e76255 into asyncapi:master Dec 19, 2022
@magicmatatjahu magicmatatjahu deleted the fix/json-schema-07 branch December 19, 2022 09:38
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 0.15.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

KhudaDad414 pushed a commit to KhudaDad414/studio that referenced this pull request Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ready-to-merge released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: monaco-yaml cannot fetch http://json-schema.org/draft-07/schema
3 participants