Skip to content

Conversation

@paulRbr
Copy link

@paulRbr paulRbr commented May 25, 2021

The internal _$refs object stores all references path with an URL
encoded value.

However when using the public API of the $refs object
most user will, most probably, expect to pass the decoded file
path to all public methods (https://github.com/APIDevTools/json-schema-ref-parser/blob/master/docs/refs.md#methods)

This commit makes sure to search for URL encoded path value when the
path is a file system path.

The internal `_$refs` object stores all references path with an URL encoded value. However when using the public API of the $refs object most user will, most probably, expect to pass the decoded file path to all public methods (https://github.com/APIDevTools/json-schema-ref-parser/blob/master/docs/refs.md#methods) This commit makes sure to search for URL encoded path value when the path is a file system path.
Copy link
Member

@philsturgeon philsturgeon left a comment

Choose a reason for hiding this comment

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

Can you add some tests to show the behavior is working as expected?

@paulRbr
Copy link
Author

paulRbr commented May 25, 2021

Thanks for pointing that to me @philsturgeon I should have done that from the start. I think this patch is incorrect and I misunderstood the library internals.

Could you help me out?

Basically, I thought I could transparently use the resulting paths from the $Refs.paths() function as inputs to the $Refs.get($ref) function. But the documentation is clear: the get($ref) function takes a “JSON Reference path, optionally with a JSON Pointer in the hash” and not a path/URL as I'm trying to do.

Maybe another solution would be to allow a new optional parameter to the $Refs.paths() function which would return encoded file path list (instead of the current decoded path list).

What do you think?

Edit: I've pushed my new suggestion here

Edit2: I guess that's what the $Refs.values() function is for. By return an object with decoded paths as keys and reference values as values. 🤦

@paulRbr
Copy link
Author

paulRbr commented May 25, 2021

I will close this PR for now as I guess I was trying to bypass the whole point of the existing $Refs.values() function (that I missed). By returning an object with decoded paths as keys, this function is exactly what you should use when searching references from their original filesystem paths. Sorry @philsturgeon for the confusion!

@paulRbr paulRbr closed this May 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants