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 the JSON library to load available variant #23

Merged
merged 1 commit into from
Apr 6, 2016

Conversation

chrisroberts
Copy link
Contributor

Hi! I'm having some downstream issues due to the way the JSON library is getting
loaded in your code (sparkleformation/sfn#178). Due to other libraries also loading
JSON and the ext variant being available, both variants end up being loaded. As a result
TypeErrors are encountered:

I tracked down an issue in the JSON repository (flori/json#269) that describes the
issue and from local testing it appears the underlying problem is a conflict between
the two implementations where each provides a subset of the entire functionality. The
result ends up being failures to generate JSON output depending on the data types. The
Fixnum type is an easy one to see fail.

From the documentation of the json project site (https://github.com/flori/json):

JSON first tries to load the extension variant. If this fails, the pure variant is loaded and used.

This is a bit more explanatory than the README file. Since the library will automatically
fallback to the pure variant if available, using require 'json' in both places is perfectly
acceptable to get the desired results. The loading bit can be seen here:

https://github.com/flori/json/blob/master/lib/json.rb#L57-L62

This would fix the issue for a number of downstream libraries and applications and
would be much appreciated to get merged and released when you can.

Thanks!

@luckymike
Copy link

👍 🙏

@trevorrowe trevorrowe merged commit 94e6706 into jmespath:master Apr 6, 2016
@trevorrowe
Copy link
Contributor

Good catch. I'll cut a patch release shortly.

@chrisroberts
Copy link
Contributor Author

Awesome, thanks so much!

@trevorrowe
Copy link
Contributor

This is available in a 1.2.4 release.

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.

3 participants