[CP] Handle object literal constructors with no library @JS annotation in dart2js

[srujzs]

Commit(s) to merge

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

Target

stable

Prepared changelist for beta/stable

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

Issue Description

JS interop allows users to declare interop extension types with object literal constructors that create objects with a given set of properties. These constructors are lowered in the backends. However, in dart2js, the code to determine whether an external constructor is an object literal constructor relied on the library having an @JS annotation. Therefore, code that did not have this annotation would fail to compile as dart2js can not determine what that member is supposed to be. The workaround is to declare the library with an @JS annotation.

This occurs only on web and in dart2js. DDC and dart2wasm do not have this issue.

What is the fix

The fix is to align with DDC and assume external extension type constructors with named arguments are object literal constructors in dart2js, regardless of whether the library contains an @JS annotation.

Why cherry-pick

This results in confusing crashes, and affects any users using interop extension types. As we encourage more Flutter packages to migrate to Wasm-compatible interop, I expect more users to be affected. This has already resulted in two bugs since 3.3 released:

#54958
#55039

The second one resulted in a broken package version accidentally being released due to the discrepancy between DDC and dart2js here.

Risk

low

Issue link(s)

#54801

Extra Info

No response

[srujzs]

Should I create a second commit targeting 3.4 beta 1, or is that handled when I submit the fix to the 3.3 stable release?

edit: Ignore, this goes into beta 2 anyways, so the cherry-pick to beta 1 isn't needed.

[itsjustkevin]

@sigmundch could you take a look at this cherry-pick request?

[sigmundch]

👍 I support this cherry-pick.

Exactly for the same reasons presented above: we have encouraged people to start migrating to use pacakge:web and they will be running into this hard to diagnose crash (multiple people have hit it so far). The localized nature of the fix makes it very low risk. It has been rolled internally for 10 days already as well.

[itsjustkevin]

@srujzs please merge to stable, we are approved!

[srujzs]

Done! Marking as closed now that it's merged.

[itsjustkevin]

@srujzs I am going to keep this open until it lands in a release, it helps me keep things together without holding the context in my head.