feat(remote_config): support nested json values

[lesnitsky]

Description

This PR adds support for nested json values

Related Issues

Closes #2074
Closes #6074

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]).
This will ensure a smooth and quick review process. Updating the pubspec.yaml and changelogs is not required.

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
• All existing and new tests are passing.
• I updated/added relevant documentation (doc comments with ///).
• The analyzer (melos run analyze) does not report any problems on my PR.
• I read and followed the Flutter Style Guide.
• I signed the CLA.
• I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

• Yes, this is a breaking change.
• No, this is not a breaking change.

[rrousselGit]

I don't think that should be the responsibility of setDefaults to encode a value. I would expect the function to be called with an already serialized parameter

[lesnitsky]

Map<String, dynamic> type of the argument suggests you can pass anything inside (which happened in #2074).
Native APIs are not symmetric, iOS does have json-related helper, android doesn't. Since we can't explicitly forbid non-primitive types on type level, I think having this helper method would be useful. An alternative would be asserting incoming value types and throwing on anything other than bool, number, or string, but I don't see this behavior as developer-friendly, especially taking into account we can provide a convenient method.

Agree, that encoding should be done not on a method channel level, so moving to packages/firebase_remote_config/firebase_remote_config/lib/src/remote_config.dart

[rrousselGit]

Since we can't explicitly forbid non-primitive types on type level, I think having this helper method would be useful

I disagree. Instead I think throwing is the way to go.

The fact that Dart type system does not allows us to statically prevent values other than primitive doesn't mean that we should handle non-primitive values.

Map<String, dynamic> is the Dart standard for JSON structures allowing only primitives.
Most function taking such Map as parameter consider non-primitive values as an error.

Using "encode" here is non-standard and is trying to patch mistakes from the users.

[lesnitsky]

The fact that Dart type system does not allows us to statically prevent values other than primitive doesn't mean that we should handle non-primitive values.

dart not allowing us to prevent values other than primitives shouldn't limit us in making the API more developer-friendly either 😉

Using "encode" here is non-standard and is trying to patch mistakes from the users.

I don't see it as a patch for a mistake. A developer's will to pass a Map as a config value is hardly a mistake (as well as willing to read that Map back via asJson)

[lesnitsky]

an alternative solution #6817

[lesnitsky]

@kroikie could you please take a look at this one and give your opinion on which API is preferable? there's also an alternative to this PR, check out #6817

[kroikie]

@lesnitsky @rrousselGit would it be possible for us to let the call go through and if accepted on the native side then we do nothing and if rejected we bubble up the possibility that if an object is being used then it should be encoded first?

[lesnitsky]

@kroikie letting the call go through is exactly what's currently happening. It leads to map serialization on java side, so {"a": ""b"} is being converted to {a=b} (or something similar, not sure how default java toString on maps works).So no error is propagated, but value is not deserialisable from that string

[kroikie]

The underlying Android and Web SDKs explicitly lists the types that can be used for defaults so I think it would be fair for us to do the same on the Dart side. So I think we should throw if the user is setting a default value that is not allowed. We should avoid transforming the user's data as much as possible.

[lesnitsky]

closing in favor of #6817