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

remove the dependency on github.com/json-iterator/go. #539

Merged
merged 1 commit into from Jan 3, 2024
Merged

remove the dependency on github.com/json-iterator/go. #539

merged 1 commit into from Jan 3, 2024

Conversation

dominiquelefevre
Copy link
Contributor

That module uses github.com/modern-go/reflect2 which is broken.

First, it is brittle because it relies on golang's implementation details that are not stable across golang releases. For example, reflect2 was broken when go 1.18 came out.

Second, reflect2 is effectively unmaintained:
modern-go/reflect2#24 (comment)

Json-iterator itself has had correctness issues like
json-iterator/go#413.

The Kubernetes project has mostly removed the dependency on json-iterator in patches like
kubernetes/kubernetes#105030.

Moreover, the Kubernetes authors found it out that json-iterator puts a lot of load on the allocator, so even the performance gains are questionable.

Let us remove dependencies on json-iterator and reflect2.

That module uses github.com/modern-go/reflect2 which is broken.

First, it is brittle because it relies on golang's implementation
details that are not stable across golang releases. For example,
reflect2 was broken when go 1.18 came out.

Second, reflect2 is effectively unmaintained:
  modern-go/reflect2#24 (comment)

Json-iterator itself has had correctness issues like
  json-iterator/go#413.

The Kubernetes project has mostly removed the dependency on json-iterator
in patches like
  kubernetes/kubernetes#105030.

Moreover, the Kubernetes authors found it out that json-iterator puts
a lot of load on the allocator, so even the performance gains are
questionable.

Let us remove dependencies on json-iterator and reflect2.
@emicklei
Copy link
Owner

emicklei commented Jan 3, 2024

thank you for contributing to this package. I will review your changes.

@emicklei emicklei merged commit 290805f into emicklei:v3 Jan 3, 2024
@michaelbeaumont
Copy link
Contributor

I think it was a mistake to remove the ability to customize MarshalIndent, NewDecoder, NewEncoder functions here and it makes it even more of a breaking change.

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.

None yet

3 participants