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

wrapAsYamlNode should maybe call .toJson() #1

Open
jonasfj opened this issue Sep 13, 2021 · 0 comments
Open

wrapAsYamlNode should maybe call .toJson() #1

jonasfj opened this issue Sep 13, 2021 · 0 comments
Labels
type-enhancement A request for a change that isn't a bug

Comments

@jonasfj
Copy link
Member

jonasfj commented Sep 13, 2021

I think the idea with wrapAsYamlNode was that you could pass it any value that json.encode (from dart:convert) would accept, and then this would wrap it as a YamlNode.

However, reading docs for jsonEncode I realize that unless some toEncodable function is given, json.encode will default to calling .toJson() on objects. This is in turn used by package:json_serializable which generates code that helps users add a .toJson() on their custom objects, making it possible to pass these objects to json.encode.

I wonder if we should consider calling .toJson() instead of throwing:

Invalid argument (value): Not a valid scalar type!: Instance of MyCustomObject
  package:yaml_edit/src/utils.dart 51:3   assertValidScalar
  package:yaml_edit/src/wrap.dart 73:5    wrapAsYamlNode
  package:yaml_edit/src/wrap.dart 126:28  new YamlMapWrap
  package:yaml_edit/src/wrap.dart 68:12   wrapAsYamlNode
  package:yaml_edit/src/wrap.dart 126:28  new YamlMapWrap
  package:yaml_edit/src/wrap.dart 68:12   wrapAsYamlNode

If we do this, we should certainly document it, and probably give an example. I doubt it is possible to gracefully test if the object has a .toJson() method, even accessing the property like object.toJson is Function is liable to throw and complain that no getter exists for toJson (if the method isn't present).

EDIT: It seems json.encode just catches all errors / exceptions when calling .toJson and then rethrows them as an exception saying: Converting object to an encodable object failed: Instance of MyCustomObject.

We could also consider adding a toEncodable function as an optional parameter similar to jsonEncode.


Migrated from google/dart-neats#76 where yaml_edit used to live.

@jonasfj jonasfj added the type-enhancement A request for a change that isn't a bug label Sep 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

1 participant