json-schema-ref-parser icon indicating copy to clipboard operation
json-schema-ref-parser copied to clipboard

Published 20 hours ago • verified publisher icon APIDevTools

#226: fix for external-from-external dependencies (exposed by addition of dereferencedCache)

Open mummybot opened this issue 4 years ago • 12 comments

This change resolves bug #226 and the PRs #146 and #223.

The if/then flow in the crawl functions of bundle and particularly dereference were such that if a match was found it would stop processing additional properties which may also contain $refs to be dereferenced.

mummybot avatar Apr 22 '21 08:04 mummybot

Sorry to poke, but will this PR be looked at? Are there changes that I need to make, or is this outside of the structure of what json-schem-ref-parser will parse?

mummybot avatar May 04 '21 12:05 mummybot

I'm not sure I understand what the bespokeKey is all about:

https://github.com/APIDevTools/json-schema-ref-parser/pull/230/files#diff-8149d22393a75a49f799c7bfdad7f3d69e6d2eb57a46eb032f11b2292fb0ea98R16-R33

philsturgeon avatar May 21 '21 09:05 philsturgeon

It highlights that a property sibling adjacent to a $ref doesn't get crawled and parsed currently. $bespokeKey is a representation of a custom keyword as allowed by Ajv, but it equally applies to the not property, which is valid JSON schema; we use a $ref for including long enum arrays.

mummybot avatar May 21 '21 09:05 mummybot

Sorry I'm trying to figure out the problems with GitHub Actions, I might have just done it, but we've got some conflicts now. Could you have a look?

philsturgeon avatar Jun 07 '21 17:06 philsturgeon

I'll fix the conflicts tomorrow.

mummybot avatar Jun 07 '21 17:06 mummybot

Conflicts resolved @philsturgeon

mummybot avatar Jun 08 '21 13:06 mummybot

Sorry to poke, any more movement on this PR? We are currently blocked from upgrading json-schema-ref-parser until this is merged.

mummybot avatar Jun 16 '21 13:06 mummybot

@P0lip could I get your input on this one? We've fixed the status checks, and there are no more conflicts, just need to figure out if this is something we want to merge.

philsturgeon avatar Jun 16 '21 18:06 philsturgeon

@P0lip sorry for chasing, but is this something which you will consider?

mummybot avatar Jun 27 '21 14:06 mummybot

Sorry to bug again, this is sitting in our backlog as a blocker and I have to justify it every morning. @P0lip / @philsturgeon is this ever going to be looked at, or shall I just close it as a "won't fix"?

mummybot avatar Jul 13 '21 12:07 mummybot

@philsturgeon will this PR be reviewed and maybe merged? We still have it outstanding in our work backlog. If not, then fine and I will close it.

mummybot avatar Nov 17 '21 08:11 mummybot

Hey, I know this has been going on for ages but I've never really understood what is happening in these tests.

What is $bespokeKey? are we adding some custom JSON Schema logic, or are you picking names that look like special characters with the intention of making sure keys with $ at the start still work?

The tests are just a bit confusing but it looks like a lot of effort has gone in here. Now that I've cleared some bugs and got the release pipeline working I am interested to get this sorted ASAP and will keep an eye out for updates.

philsturgeon avatar May 11 '22 21:05 philsturgeon

I'm closing stale PRs - happy to merge in a new PR if it's rebased

jonluca avatar Sep 18 '23 23:09 jonluca

God, it is so long ago that I looked at this, and I missed @philsturgeon's last message back in May 2022 :( I've no idea if this is still an issue, but the rebase is now horrendous, and I've moved projects.

If I encounter this issue again on a new project, I'll pick this up. A shame it didn't get added.

mummybot avatar Sep 19 '23 20:09 mummybot