[breaking change] Remove `Object` from `ExternalDartReference`'s implemented types

[srujzs]

Change Intent

dart:js_interop's ExternalDartReference allows users to pass opaque references to Dart objects such that they can be passed through JavaScript back to Dart efficiently. The library provides two extension methods: toExternalReference and toDartObject to convert to and from the opaque reference.

ExternalDartReference will now instead be generic and toExternalReference will return a reference to any T, including null. Therefore, the extension type can longer implement Object, as the representation type will be nullable.

Justification

ExternalDartReference should be generic to avoid extra downcasts when compiling to JS, and to be able to represent what type of Dart object the reference refers to.

On top of this, we should include null values i.e. T extends Object? instead of T extends Object, so that other generic interfaces e.g. Finalizer that have an unbounded T do not have to include extra null-checks to be able to convert to and from an ExternalDartReference.

Original issues: https://github.com/dart-lang/sdk/issues/55536, https://github.com/dart-lang/sdk/issues/55342

Impact

Low. There is low usage in two repos:

1. Flutter engine in order to use the JS FinalizationRegistry. 0 1 2 3
2. svelte_js.dart. The owner is the same user who originally filed the request.

Mitigation

Any assignment of ExternalDartReference to Object can instead be made an assignment to Object?. For example:

// Object o = 0.toExternalReference;
Object? o = 0.toExternalReference;

// void f(Object o) {}
void f(Object? o) {}

f(0.toExternalReference);

Change Timeline

The implementation is already done (see below). Ideally, this should go in beta 2 so that we don't have to wait for the next stable to submit this as breaking changes can't be submitted in the last beta.

Associated CLs

https://dart-review.googlesource.com/c/sdk/+/370663

[srujzs]

@itsjustkevin I sent out an email to announce@dartlang.org for this. Let me know if you need anything else to start the review process. Thanks!

[kevmoo]

LGTM!

[sigmundch]

Thanks @srujzs. Breaking change LGTM too.

cc @mkustermann

[mkustermann]

LGTM

As the API that's being changed is relatively new with limited users, I'd suggest to use an accelerated process to ensure the breaking change will land before the next stable (to minimize change of more users to start relying on the now-deprecated API)

[mkustermann]

@mit-mit @vsmenon The cutoff for next stable seems to be 1st of July. Can we get this approved beforehand to ensure it's included in the next stable? (If we delay this, people may start relying on this API more widely and changing it will cause more trouble for ecosystem, so the sooner we do it the better)

[mit-mit]

LGTM

[mit-mit]

Please make sure to post on dart-announce

[sigmundch]

Thanks, it was posted last week. Usage currently is extremely minimal, so we don't expect to hear much.

[mkustermann]

@itsjustkevin Does it need signoff from anyone else or can this be marked as approved and @srujzs can go ahead and land the change?

[sigmundch]

FYI @itsjustkevin is OOO this week. We believe he will be back next week.

[sigmundch]

Looking at the updated calendar, it seems @itsjustkevin will be OOO until mid week. I reviewed previous breaking change requests and it appears we typically seek approval from representatives in Flutter and ACX as well.

@Hixie @leonsenft @vsmenon - could you take a look at this breaking change request? Thank you!

[leonsenft]

LGTM 👍

[Hixie]

no objection from me. cc @yjbanov

[vsmenon]

lgtm

[sigmundch]

Thanks everyone! Following the protocol, I went ahead and marked this breaking change as approved

@itsjustkevin - when you are back, is there anything else do to for this issue or is it OK to close it now?

@srujzs - we should be OK to move forward now ahead of the upcoming cutoff next week