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

allow JSON schema 2019-09 as well as draft-07 #81

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fmigneault
Copy link

@fmigneault fmigneault commented Nov 6, 2024

@m-mohr
Copy link
Collaborator

m-mohr commented Nov 6, 2024

Most of the ecosystem (except MLM, it seems) is draft-07. ajv for draft-2019 adds quite a bit of overhead size (according to the ajv docs) so it needs to be checked whether we can implement it in a way that the bundle size doesn't increase too much while trying to make it flawslessly work in Node. Maybe in a way that it dynamically loads 2019 if needed and you can opt out of it for browsers and if that is the case the report states "unknown" for non-draft-07 schemas. I'll also need to check how much overhead it actually is to use 2019, but the ajv docs indicate overhead. So we can't merge this as is unfortunately, it needs more checks and work upfront. Unfortunately, I won't have the time for it this year but I'd be happy to review if you have time to work on it.

Also note that there's a v2 branch, which we also need to update and my comment in stac-extensions/mlm#52 (comment).

PS: I actually also tested 2019 integration a couple of days ago for v2, but couldn't get it working in an easy way so had to stop it for now. I'll revisit this next year, but it's not of high priority right now.

@fmigneault
Copy link
Author

fmigneault commented Nov 6, 2024

I'm not sure if there's a good way to dynamically load ajv2019, unless there's an explicit option passed to the command line to enable it? Would that be a good approach?

Figuring out which schema to use is a chicken-and-egg situation. You need to load the schema to see if $schema references it, but the loader is defined when creating the ajv object.

Running it, I do see a certain overhead.

@m-mohr
Copy link
Collaborator

m-mohr commented Nov 6, 2024

The dynamic loading is a browser thing, so there are no CLI options.

It's not really a chicken-and-egg problem, I think. I can interfere loading the schema. So if I see that it's 2019 I can either skip it or try to load 2019. I just didn't implement it yet as I wasn't aware that unknown schemas lead ajv to just load endlessly. If that's not due to our custom schema loader, I'd even consider this behaviour being a bug. ajv should throw an error and not just hang...

@fmigneault
Copy link
Author

OK. I can give it a try.

@m-mohr
Copy link
Collaborator

m-mohr commented Nov 6, 2024

We should first check how much overhead it actually is in the browser, otherwise you may just waste time. If the overhead is marginal (and I mean more the bundle size than the validation time, although that also shouldn't be like 2x slower or so) we can also just load 2019...

@m-mohr m-mohr marked this pull request as draft November 7, 2024 00:31
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.

Validator hangs with other $schema than draft-07
2 participants