Skip to content

Add Reviver #679

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add Reviver #679

wants to merge 4 commits into from

Conversation

Nexushunter
Copy link

@Nexushunter Nexushunter commented Sep 19, 2023

Motive

I work on a project to provide a dart annotation that allows us to run the OpenAPI generator from a Dart defined config.

I was looking to provide a clean way to test our annotation and ensure that consumer could use a custom header delegate. In this case I wanted an easy way to be able to generically restore a header delegate that a user may provide but would like to have been able to avoid needing to know about their implementation details. ie User creates a subclass, they use the builder we provide and our builder needs to know nothing of their implementation details.

To help facilitate this it would be nice if the source_gen library had a way to revive the object in memory allowing for us to clean separate ownership (external library function vs internal library function).

While I know that the dart:mirrors library is classified as unstable it would be very beneficial to be able to have a solution provided to the community. This would also help with not having the need (though they could still be used) for packages like generic_reader.

Changes

  • Add a Reviver class that uses dart:mirrors to reflect a ConstantReader / DartObject? into an in memory representation.

  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

Comment on lines +174 to +188
List<T>? toTypedList<T>(T t) => reader.listValue
.map((e) => Reviver.fromDartObject(e).toInstance() as T)
.toList() as List<T>?;

Map<KT, VT>? toTypedMap<KT, VT>(KT kt, VT vt) => reader.mapValue.map(
(key, value) => MapEntry(
Reviver.fromDartObject(key).toInstance() as KT,
Reviver.fromDartObject(value).toInstance() as VT,
),
) as Map<KT, VT>?;

Set<T>? toTypedSet<T>(T t) => reader.setValue
.map((e) => Reviver.fromDartObject(e).toInstance() as T)
.toSet() as Set<T>?;
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying to leverage these results in the following stack trace:

dart:collection                                       MapBase.map
package:source_gen/src/constants/reviver.dart 178:68  Reviver.toTypedMap
package:source_gen/src/constants/reviver.dart 93:16   Reviver.toInstance
package:source_gen/src/constants/reviver.dart 142:51  Reviver.namedValues.<fn>
dart:collection                                       MapBase.map
package:source_gen/src/constants/reviver.dart 139:74  Reviver.namedValues
package:source_gen/src/constants/reviver.dart 109:18  Reviver.toInstance
package:source_gen/src/constants/reviver.dart 135:50  Reviver.positionalValues.<fn>
dart:_internal                                        ListIterable.toList
package:source_gen/src/constants/reviver.dart 137:8   Reviver.positionalValues
package:source_gen/src/constants/reviver.dart 108:18  Reviver.toInstance
test/constants/reviver_test.dart 56:38                main.<fn>.<fn>.<fn>.<fn>

This feels wrong. The reason I'm doing this is because there is a limitation of not being able to do:

final t = Reviver.fromDartObject(reader.listValue.first)
              .toInstance()
              .runtimeType;

return reder.listValue.map((e) => Reviver.fromDartObject(e).toInstance() as t).toList();

@Nexushunter
Copy link
Author

Opening this early to get some feedback and thoughts.

@Nexushunter Nexushunter marked this pull request as ready for review September 24, 2023 18:19
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant