dart-lang / yaml_edit

A library for YAML manipulation with comment and whitespace preservation.
https://pub.dev/packages/yaml_edit
BSD 3-Clause "New" or "Revised" License
27 stars 16 forks source link

wrapAsYamlNode should maybe call .toJson() #1

Open jonasfj opened 3 years ago

jonasfj commented 3 years ago

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 https://github.com/google/dart-neats/issues/76 where yaml_edit used to live.