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

Deprecate TypeSafeWrappers? #588

Closed
matanlurey opened this issue Mar 12, 2018 · 5 comments · Fixed by dart-archive/collection#86
Closed

Deprecate TypeSafeWrappers? #588

matanlurey opened this issue Mar 12, 2018 · 5 comments · Fixed by dart-archive/collection#86
Labels
package:collection type-question A question about expected behavior or functionality

Comments

@matanlurey
Copy link
Contributor

matanlurey commented Mar 12, 2018

... given that Dart 2 does this out of the box?

Here's what public APIs package offers today:

Here's an example of the implementation of those static methods:

https://github.com/dart-lang/collection/blob/fac9e50be7e61819e66abfd6381d55135b8751d3/lib/src/wrappers.dart#L148-L149

I imagine to avoid churn, we could do the following:

  • Add a package-internal field called isStrongMode:
final bool isStrongMode = (() {
  bool typeParamIsString<T>(T item) => T == String;
  return typeParamIsString('');
})();
  • Change the .typed methods to be:
static List<E> typed<E>(List base) {
  if (isStrongMode) {
    return base.cast<E>();
  }
  return base is List<E> ? base : new TypeSafeList<E>(base);
}
  • When Dart 2 is enabled by default everywhere (Dart2JS/VM/Flutter), remove the TypeSafeX wrapper classes, as they will never be used. This should not be a breaking change, strictly.

  • At the same time of above, deprecate the .typed methods, because they no longer have value and are misleading, doing is and .cast is fine for all users at this point.

Thoughts? @lrhn @srawlins @kevmoo

@matanlurey matanlurey added the type-question A question about expected behavior or functionality label Mar 12, 2018
@nex3
Copy link
Member

nex3 commented Mar 13, 2018

As long as we're only supporting Dart 2 (which appears to be the case based on the current pubspec), we can do this much more simply by just using cast() everywhere and deleting the TypeSafe wrappers.

@matanlurey
Copy link
Contributor Author

Definitely, I just am trying to be nice to users - in my own tests it looks like the type of errors thrown by Dart2 versus thrown currently by the wrappers are not exactly the same.

@nex3
Copy link
Member

nex3 commented Mar 13, 2018

How much does that buy us in practice? I don't see how code could ever actually construct a TypeSafeList with your proposed intermediate implementation.

@matanlurey
Copy link
Contributor Author

Until the VM and Dart2JS enforce strong mode, strong-mode typed lists will not throw. So you could potentially be using .typed for safety, and it throws in DDC but not Dart2JS.

@nex3
Copy link
Member

nex3 commented Mar 13, 2018

But if dart2js isn't using strong mode semantics, then base is List<E> will always be true (because <E> is erased) and nothing will throw anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:collection type-question A question about expected behavior or functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants