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

[dart2wasm] Remove support for package:js and dart:js_util in favor of dart:js_interop and dart:js_interop_unsafe #54004

Closed
srujzs opened this issue Nov 9, 2023 · 2 comments
Assignees
Labels
area-dart2wasm Issues for the dart2wasm compiler. area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. web-js-interop Issues that impact all js interop

Comments

@srujzs
Copy link
Contributor

srujzs commented Nov 9, 2023

package:js and dart:js_util is not restrictive enough for our purposes and may lead to unexpected behavior (either due to odd semantics or lack of support) and/or performance cliffs. This will require migrating internal usage, but for now we should treat these two libraries as deprecated for dart2wasm. Support for these libraries will continue on dart2js and ddc until we feel ready to start deprecating and migrating.

This will require migrating any Flutter usages to use dart:js_interop, but we can allowlist existing usages for now and disallow these libraries in dart2wasm for 3.3.

This will also require making sure dart:js_interop reaches parity with package:js and dart:js_util by providing any missing functionalities.

cc @kevmoo

@srujzs srujzs added web-js-interop Issues that impact all js interop area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. area-dart2wasm Issues for the dart2wasm compiler. labels Nov 9, 2023
@srujzs srujzs self-assigned this Dec 12, 2023
copybara-service bot pushed a commit that referenced this issue Dec 25, 2023
#54004

In order to reach parity with js_util, we expose operators
through an extension and do some partial renames in order
to make the member names sound better e.g. `equals` instead
of `equal`. We also expose the following from js_util:

- NullRejectionException

We don't expose `isJavaScriptArray` and `isJavaScriptSimpleObject`
as they can expressed through other forms of interop. There
was an esoteric bug where we needed these members for Firefox
in pkg:test, but the package no longer uses these members, so to
avoid increasing the API space too much, we do not export them.

For the same reason, we also don't expose `objectGetPrototypeOf`,
`objectPrototype`, `objectKeys`.

We don't expose `allowInteropCaptureThis` as it will take some
work to handle this in dart2wasm. That work is tracked in
#54381.

Lastly, `instanceof` and `instanceOfString` is moved to be on
`JSAny?`, as this operator is useful to avoid needing to
downcast to `JSObject` e.g. `any.instanceOfString('Window')`
instead of `any.typeofEquals('object') &&
(any as JSObject).instanceOfString('Window')`.

Extensions are reorganized and renamed to handle these changes.

CoreLibraryReviewExempt: Backend-specific library.
Change-Id: Ib1a7fabc3fa985ef6638620becccd27eeca68c25
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/341140
Reviewed-by: Sigmund Cherem <sigmund@google.com>
copybara-service bot pushed a commit that referenced this issue Dec 28, 2023
This reverts commit 8c246ca.

Reason for revert: Need to allowlist benchmarks to use package:js.

Original change's description:
> [dart2wasm] Disallow use of old interop libraries
>
> Closes #54004
>
> Adds an error for imports of old interop libraries. Has an
> allowlist for existing usages/migrated usages that we'll need
> to migrate.
>
> Change-Id: Ie7174ae2a50c2d03a7aa2975e8a1914a4cba8a2c
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/342521
> Reviewed-by: Sigmund Cherem <sigmund@google.com>
> Commit-Queue: Srujan Gaddam <srujzs@google.com>

Change-Id: Id7d3a3063d417830774d7a72eed5185599f9267f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/343685
Reviewed-by: Alexander Thomas <athom@google.com>
Commit-Queue: Alexander Aprelev <aam@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Jonas Termansen <sortie@google.com>
@srujzs srujzs reopened this Dec 28, 2023
@srujzs srujzs closed this as completed Jan 3, 2024
@parlough
Copy link
Member

parlough commented Jan 3, 2024

For reference for those viewing the issue, the commit that fixed this the second time was 3a80ec2.

Thanks Srujan! This work will help making migration easier and clearer for developers trying out Wasm compilation.

@srujzs
Copy link
Contributor Author

srujzs commented Jan 3, 2024

Thanks! I wish we had Github support to handle reverts and relands (essentially automatically reopening and reclosing the issue), but this is likely pretty complicated to support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-dart2wasm Issues for the dart2wasm compiler. area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. web-js-interop Issues that impact all js interop
Projects
None yet
Development

No branches or pull requests

2 participants