[breaking change] No longer allow extending HtmlElement and other native types.

[sigmundch]

Change Intent

We intend to disallow extending HtmlElement and other native types, this entails removing the .created constructors for types in dart:html and eventually marking those classes with the interface class modifier.

Justification

Historically, all native types were sealed within the SDK and couldn't be extended (using tricks like only providing factory constructors publicly). This was relaxed only to support custom elements v0.5 long time ago. Last year we deprecated and removed support for custom element (see #49536).

Now we would like to do some additional cleanup in our compilers to remove technical debt related to custom elements. To ensure users don't accidentally trip over old unsupported logic, we would like to make it a static error to use the unsupported patterns. Namely, that would be to make it a static error to extend from HtmlElement going forward.

Impact

Code like:

class A extends HtmlElement {
  A.created() : super.created();
}

will now give a compile-time error.

These classes could only be used in combination with the registerElement API that was removed with Dart 3.0, because of that we believe the impact of this change is limited. The change can affect only developers that removed calls to registerElement but left intact the classes associated with those calls on the previous breaking change. Note that those classes cannot be instantiated anymore, as such we think the impact of this change is pretty low. Given the correlation of the two APIs, we believe it will be rare for developers to encounter the breaking change.

Mitigation

The recommended mitigation is to remove the subclass,. In the rare case that such subclasses were used for mocking in tests, developers can use implements instead of extends to achieve their goals.

[itsjustkevin]

@sigmundch do you have a proposed timeline for possibly breaking users here? Will the method be deprecated for a while before it is removed?

[sigmundch]

Even though this is a breaking change, we hope it is effectively not breaking because of the previous breaking change we did to custom elements. Because of that, we'd like to assess whether we can land this breaking change w/o a deprecation period. We will validate that indeed has the limited impact that we predict (both by looking at internal systems and waiting for community feedback), and if all goes well, we can hopefully land this in the next release.

[kevmoo]

SGTM!

[sigmundch]

quick update: an internal global presubmit of the breaking change had no failures. Implementation for the breaking change is here: https://dart-review.googlesource.com/c/sdk/+/321741

[sigmundch]

friendly ping :)

[itsjustkevin]

@Hixie @vsmenon @grouma

[vsmenon]

lgtm

[grouma]

LGTM

[Hixie]

fine by me

[kevmoo]

What's the story here?

[sigmundch]

Unfortunately I think I dropped the ball here. We made the public breaking change announcement and, as we expected, there was no concern expressed. I believe we are good to move forward, we just need to rebase the change and push it though.

@natebiggs @srujzs - do you recall any other concerns, if not, would you be willing to take over the CL to push it through?

@kevmoo - I'm curious if you have an additional motivation here. On our end, the main benefit was to clean up tech debt in dart2js. Now that the entire library is marked as deprecated, the prioritization may be different.

[biggs0125]

Seems reasonable to push forward if we have good motivation. I'm not sure it makes sense to take on the additional cleanup right now. But given we have the CL and it looks like it shouldn't be too painful to rebase, we should go ahead and land it now. That way if we ever do choose to follow up with any of the extra custom element cleanup this doesn't become a blocker again.

[srujzs]

I have no concerns here - I can go ahead and rebase the CL, do a TGP, and then land.

[sigmundch]

thank you!

[srujzs]

https://dart-review.googlesource.com/c/sdk/+/321741 has landed. @itsjustkevin I'll leave this open for your tracking purposes, but this should be resolved.

[sigmundch]

Thanks so much for pushing it through!