Revert dropping MultiJson to fix Time serialization regression#405
Merged
Revert dropping MultiJson to fix Time serialization regression#405
Conversation
PR #385 replaced MultiJson.dump with JSON.dump which bypasses ActiveSupport's Hash#to_json override, causing Time values to serialize as "2026-04-16 10:55:10 UTC" instead of ISO 8601. Restore direct multi_json dependency and MultiJson.dump call. Remove the intermediate Grape::Entity::Json constant introduced by #385 as it is no longer needed. Fixes #403
Danger ReportNo issues found. |
There was a problem hiding this comment.
Pull request overview
Reverts the MultiJson removal from #385 to restore the pre-1.0.3 Time serialization behavior (ISO 8601 via ActiveSupport’s as_json) and prevent silent breaking changes in API responses.
Changes:
- Restored
MultiJson.dumpforGrape::Entity#to_jsonand reintroducedmulti_jsonas a runtime dependency. - Removed the
Grape::Entity::Jsonshim and its spec since it is no longer used. - Added regression specs to assert
Timeserialization viaas_jsonfor both flat and nested exposures.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| spec/grape_entity/json_spec.rb | Removes spec for the deleted Grape::Entity::Json shim. |
| spec/grape_entity/entity_spec.rb | Adds regression coverage for Time serialization (flat + nested). |
| lib/grape_entity/json.rb | Deletes the no-longer-used JSON backend shim. |
| lib/grape_entity/entity.rb | Restores MultiJson.dump in #to_json and requires multi_json. |
| grape-entity.gemspec | Re-adds multi_json as a dependency to support restored behavior. |
| CHANGELOG.md | Documents the regression fix/revert for the upcoming release. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This reverts the behavioral changes from #385 ("Drop multijson dependency") which introduced a silent breaking change in 1.0.3: all
Timefields in API responses changed format for applications using ActiveSupport."2026-04-16T10:55:10.597Z"(ISO 8601)"2026-04-16 10:55:10 UTC"The goal is to restore the exact serialization behavior from 1.0.1 for a patch release, so affected consumers can upgrade immediately without changes on their side.
What went wrong
PR #385 replaced
MultiJson.dumpwithJSON.dump(via aGrape::Entity::Jsonshim) inEntity#to_json. These two methods behave differently when ActiveSupport is loaded:MultiJson.dump(hash)internally callsHash#to_json, which ActiveSupport overrides to runas_jsonrecursively — givingTimeits ISO 8601 representation.JSON.dump(hash)goes directly to Ruby's C extension JSON generator, bypassing ActiveSupport's override — soTimefalls back toTime#to_s.Since
activesupportis a runtime dependency of grape-entity, every user was affected.Additionally, the
Grape::Entity::Jsonshim from #385 checkeddefined?(::MultiJson)at require time to decide which backend to use. For applications withgem "multi_json", require: false, MultiJson was never loaded at that point — so the constant always resolved to::JSON, making the MultiJson detection dead code in practice (as noted by @stevenou in #403).What this PR does
require 'multi_json'andMultiJson.dumpinEntity#to_json— the same code path that was in place at 1.0.1multi_jsonas a gemspec dependencyGrape::Entity::Jsonshim (lib/grape_entity/json.rb) and its spec, as they are no longer referencedTimevalues serialize viaas_jsonin both flat and nested exposuresA separate PR (#404) explores dropping MultiJson entirely without the regression by using
Hash#to_jsondirectly — that is a candidate for a future minor release.Fixes #403