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 raise_cannot_infer_root_key_error to config #2412

Conversation

tashirosota
Copy link

@tashirosota tashirosota commented Feb 11, 2021

Purpose

  • Raise error when cannot infer root key by v10.0.7 . I think this is braking change.
  • Added config that you can upgrade without changing existing code.

Changes

  • ArgumentError changes to CannotInferRootKeyError in CollectionSerializer#json_key
    • Custom error helps clarify the problem
  • Add config.raise_cannot_infer_root_key_error
  • Add test when serializer does not have type and collection is empty

Related GitHub issues

Our project has many like this code😢

::ActiveModelSerializers::SerializableResource.new(
  collections, # [] empty array
  each_serializer: NonTypeCollectionSerializer,
).serializable_hash || []
※ v0.10.6

@tashirosota tashirosota changed the title add raise cannot infer root key error to confid Add raise_cannot_infer_root_key_error to config Feb 11, 2021
@wasifhossain
Copy link
Member

Looks good to me

@tashirosota
Copy link
Author

@wasifhossain
Will this PR be merged?

@wasifhossain
Copy link
Member

let's do this

@wasifhossain wasifhossain merged commit 01c0d69 into rails-api:0-10-stable Mar 3, 2021
@wasifhossain
Copy link
Member

thanks for the reminder!

@mvz
Copy link

mvz commented Mar 3, 2021

This doesn't seem to resolve the problem that this error is also raised when each_serializer is specified, even though specifying an each_serializer is suggested as a way to resolve the error.

@tashirosota
Copy link
Author

tashirosota commented Mar 4, 2021

@mvz
When each_serializer is specified and don't want to make an error.
Can set ActiveModelSerializers.config.raise_cannot_infer_root_key_error =false and not raise error.
This error coused when each_serializer is specified and (serializer's type is null || collection is empty || root key is null), it is correct behavior, but braking change.
So I patched this option, resolve to this case < v0.10.6

::ActiveModelSerializers::SerializableResource.new(
  collections, # [] empty array
  each_serializer: NonTypeCollectionSerializer,
).serializable_hash || []

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