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

Add Jsonify support for optional object keys #445

Merged
merged 4 commits into from
Aug 26, 2022

Conversation

skarab42
Copy link
Collaborator

This PR adds support for optional object keys

Related to #424

Verified

This commit was signed with the committer’s verified signature.
skarab42 Sébastien Mischler

Verified

This commit was signed with the committer’s verified signature.
skarab42 Sébastien Mischler

Verified

This commit was signed with the committer’s verified signature.
skarab42 Sébastien Mischler

Verified

This commit was signed with the committer’s verified signature.
skarab42 Sébastien Mischler
@skarab42 skarab42 changed the title Add Jsonify support for optional object keys #424 Add Jsonify support for optional object keys Aug 26, 2022
@sindresorhus
Copy link
Owner

// @frontsideair

Copy link
Contributor

@frontsideair frontsideair left a comment

Choose a reason for hiding this comment

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

I'm really happy with the changes and the current test coverage. I also plugged this branch to Remix and it fixes relevant tests. (There are only two test cases that are failing, irrelevant to this PR. For those curious: "supports plain Response" and "transforms unserializables to null in arrays".)

Let's go ahead and merge this. Thanks a lot for picking this up and great delivery!

@sindresorhus sindresorhus merged commit d83d62c into sindresorhus:main Aug 26, 2022
@skarab42 skarab42 deleted the feat-jsonify-optional branch August 28, 2022 08:10
@pcattori
Copy link
Contributor

pcattori commented Sep 2, 2022

I also plugged this branch to Remix and it fixes relevant tests. (There are only two test cases that are failing, irrelevant to this PR. For those curious: "supports plain Response" and "transforms unserializables to null in arrays".)

@frontsideair : to be clear, were there failing tests for this in the Remix codebase? AFAIK, we've only been merging PRs with all passing tests (with minor exceptions for Windows compatibility tests).

Or are the failing tests you are referring to elsewhere?

@frontsideair
Copy link
Contributor

Sorry for the confusion, I was talking about using Jsonify from this repo instead of the one in Remix repo, which I hope will replace it soon. There are no failing test in Remix afaict.

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

4 participants