pana icon indicating copy to clipboard operation
pana copied to clipboard

Detect and reward packages that are wasm compatible

Open kevmoo opened this issue 1 year ago • 27 comments

Repurposing this issue to track a specific platform/label of "good" wasm users. This will allow giving "bling" to folks on the package site.

kevmoo avatar Jan 31 '24 23:01 kevmoo

Can link to http://dart.dev/go/package-web which will point to "the right docs" when we're ready

kevmoo avatar Feb 01 '24 01:02 kevmoo

It seems a bit premature to take points away before the old APIs are clearly marked as deprecated and new APIs are marked stable?

mit-mit avatar Feb 06 '24 12:02 mit-mit

We've already added notes to the legacy APIs - https://api.dart.dev/main/dart-html/dart-html-library.html

We want a signal on pub first. Deprecations are loud and tend to cause lots of issues for the world. We just want to put (a bit) of pressure on the ecosystem.

We are not taking ANY points away. We're just giving the opportunity to earn more points for "doing the right thing" 😄

kevmoo avatar Feb 06 '24 15:02 kevmoo

Had a great chat with @isoos and @jonasfj about this. Going to repurpose this request to track packages that are "wasm ready" as a label and then give them some "bling" on the package site.

kevmoo avatar Feb 07 '24 21:02 kevmoo

Everyone will want some of the Wasm bling! I definitely like that more than reducing points as they can be a source of frustration for developers.

parlough avatar Feb 07 '24 22:02 parlough

Like we could look into using the Wasm logo or something.

image

kevmoo avatar Feb 07 '24 23:02 kevmoo

Is this still a P1 issue?

mosuem avatar Mar 12 '24 13:03 mosuem

I'd like some of the false positives/negatives to be prioritized. maybe not p1, though.

kevmoo avatar Mar 12 '24 19:03 kevmoo

@kevmoo, Can we try to flesh the remaining work out a bit.

What false positives/negatives do we know?

Currently a package gets is:wasm-ready iff:

  • all dart: libraries transitively reachable in the import graph from the main library belongs to the set
    • 'dart:async',
    • 'dart:collection',
    • 'dart:convert',
    • 'dart:core',
    • 'dart:developer',
    • 'dart:math',
    • 'dart:typed_data',
    • 'dart:_internal',
    • 'dart:ui',
    • 'dart:ui_web',
    • 'dart:js_interop',
    • 'dart:js_interop_unsafe'

Is that the right rule?

Do we have any other things we want to affect the decision?

sigurdm avatar Apr 02 '24 09:04 sigurdm

@sigurdm – that sounds right, but we should look at the items at https://github.com/dart-lang/pub-dev/issues/6785#issuecomment-1947947416

We should also exclude imports of pkg:js

kevmoo avatar Apr 02 '24 17:04 kevmoo

I don't know if allowlisting is easier vs denylisting, but from the interop side, anything from the "previous libraries" section here: https://dart.dev/interop/js-interop#next-generation-js-interop is considered not Wasm-compatible.

srujzs avatar Apr 02 '24 21:04 srujzs

we should look at the items at https://github.com/dart-lang/pub-dev/issues/6785#issuecomment-1947947416

Ah sorry - somehow overlooked this list:

intl: false positive The package still seems to use dart:html

This has migrated. I checked out 709bdcd4 to investigate.

The import is in lib/intl_browser.dart:12:import 'dart:html'; not the "main library" (named the same as the package) that is why we still assign the tag.

i18n_extension: false positive Also, packages depending on intl are listed as supported.

~/p/i18n_extension (master)> git grep 'package:intl'
lib/src/i18n_widget.dart:6:import 'package:intl/number_symbols.dart';
lib/src/i18n_widget.dart:7:import 'package:intl/number_symbols_data.dart';

It doesn't import the offending file from the intl package -> it gets the tag.

flutter_keyboard_visibility: false positive Depends on dart:html

Only depends on dart:html from: https://github.com/MisterJimson/flutter_keyboard_visibility/blob/42ddef531e437cdd15b7baf270b64c8ae8ce8a28/flutter_keyboard_visibility_web/lib/flutter_keyboard_visibility_web.dart#L4

Not from the main library file (the one named the same as the library).

intercom_flutter: false positive Depends on dart:js and dart:html

No longer true.

I checked out the last version containing the imports 6af2a0c8, and I got the following tags:

 "tags": [
  "sdk:flutter",
  "platform:web",
  "is:plugin",
  "is:null-safe",
  "is:dart3-compatible",
  "license:mit",
  "license:fsf-libre",
  "license:osi-approved"
 ],

Seems OK

oauth2_client: false positive: Depends on a flutter_secure_storage which depends on flutter_secure_storage_web which depends on package:js

This one is interesting. The flutter_secure_storage_web package is a federated plugin. I believe their import relies on code-generation (@jonasfj is this true?) I don't know how we would handle that.

Also the federated plugin structure means that you could potentially replace flutter_secure_storage_web with another package that was not importing package:js. So is this really a strict positive?

sensors_plus: false positive: Depends on both dart:html and dart:js_util

Another interesting one: https://github.com/fluttercommunity/plus_plugins/blob/main/packages/sensors_plus/sensors_plus/lib/sensors_plus.dart#L7 has a conditional import of the web-part. But that is conditioned on dart:html.

When compiling for WASM the (dart:html) condition will be false, and thus the file will not be imported. Thus the package is broken on WASM, but we don't detect it.

To me that shows a weakness of the conditional import system.

Not sure how we should handle this.

uni_links: false positive: Depends on dart:html

Not the main lib.

fetch_client: false negative ??? There is a pre-release version with WASM support, but still listed as unsupported? I guess that might be expected due to the pre-release

Indeed. The we index the latest stable (non-prerelease) version.

We should also exclude imports of pkg:js

Those are already excluded (that is the second 'f' of iff).

sigurdm avatar Apr 04 '24 07:04 sigurdm

intl: false positive

package:intl is WIP. I will try to reland the switch to WASM soon. See https://github.com/dart-lang/i18n/pull/822

mosuem avatar Apr 04 '24 08:04 mosuem

It indeed seems detecting wasm-incompatibility for federated plugins will be hard. Flutter generates code that imports the platform specific implementations -> we cannot just follow import paths.

https://github.com/flutter/flutter/blob/1a0dc8f1e11892f8f2365ffb6a9617abc39d4b15/packages/flutter_tools/lib/src/build_system/targets/dart_plugin_registrant.dart#L30

But one could argue that a federated plugin is wasm-ready (while it's default web option might not be). But that is of course mostly being technically correct...

sigurdm avatar Apr 29 '24 13:04 sigurdm

But that is of course mostly being technically correct...

Note: a similar case happened with my docker_process package: somebody wanted to know how it runs on Android (and iOS). Technically it executes the docker executable, but in practice it won't do it, as it won't usually run on mobile phones. I am a bit undecided if at all we could detect that, or should we just suggest the package author to fill out platforms: in the pubspec. If the later, we should have a similar entry for wasm maybe?

isoos avatar Apr 29 '24 14:04 isoos

WASM support is stable with Flutter 3.22.0. I would have expected this to be ready for the release or even well before so developers know what packages work with it and package maintainers get nudged to update. Has there been any progress?

Rexios80 avatar May 18 '24 12:05 Rexios80

@Rexios80: Have you checked this search filter on pub.dev?

image

isoos avatar May 18 '24 12:05 isoos

That is not very discoverable, there isn't a badge on compatible packages, and it doesn't do anything to incentivize package maintainers to support WASM. I think adding another 10 points for WASM support would be much preferable as that is a strong nudge to migrate. Are there any APIs dart:html has that are not supported with the new web interop libraries? If there aren't I see no reason to avoid docking points on packages that haven't migrated.

Rexios80 avatar May 18 '24 12:05 Rexios80

Currently pub.dev has badges for:

  • SDK: Flutter | Dart
  • Platform: Windows | Android | ... | Web

We don't have badges, but we do have search filters (if you read help pages) for:

  • Runtime: AOT | JIT | JS

I think wasm is more like a runtime than a platform. Hence, I don't think we know how we want to add it in the UI.

We could add a different kind of badge: wasm-ready, like we had for null-safety, before it became the norm. Maybe, that's a path. It's just we'll be spamming all the pure-Dart packages that don't interface js/html with this badge too.


I think granting 10 points or something like that for packages that are wasm-ready, in the platform scoring section might be reasonable.

Not sure was @kevmoo thinks about all this.

jonasfj avatar May 18 '24 23:05 jonasfj

The thing that makes most sense is flagging legacy-html honestly. And giving 5-10 points for packages that AVOID legacy-html imports.

That's my technical answer, but I'd want to brainstorm the right "community/ecosystem" way to do this so we don't hurt folks feelings.

kevmoo avatar May 19 '24 01:05 kevmoo

The thing that makes most sense is flagging legacy-html honestly.

I think we should be careful flagging it just yet. I mean it'll be a LONG time before dart:html goes away, right? (if ever)


But I think that giving 5-10 points for avoiding dart:html and friends is a great solution.

And giving 5-10 points for packages that AVOID legacy-html imports.

That probably includes avoiding transitive imports of legacy-html.

jonasfj avatar May 21 '24 12:05 jonasfj

Is there a reason for dart:html to stick around besides maintainers not migrating? I see no reason to not penalize them for not keeping up to date with Dart best practices.

Rexios80 avatar May 21 '24 13:05 Rexios80

Is there a reason for dart:html to stick around besides maintainers not migrating?

I think it's safe to assume that not all migrations are trivial.

There are some very large Dart code bases out there. Migrating them might take years. It might be easier to rely on backwards compatibility, even if you don't get some of the benefits of the new interop model.

jonasfj avatar May 21 '24 14:05 jonasfj

Is there a reason for dart:html to stick around besides maintainers not migrating? I see no reason to not penalize them for not keeping up to date with Dart best practices.

It is worth to keep in mind that life happens to all of us, and not everyone is ready to put effort in migrating on day one (or day 20). If you need a package to be migrated, please, by all means reach out to its developers, and help them to do so. API deprecation and with that some kind of penalty may happen down the line, but I don't see any reason to rush it.

isoos avatar May 21 '24 14:05 isoos

I don't see any reason to not rush it. WASM support is touted as "stable" in Flutter 3.22.0 and yet six out of seven of my web projects fail to build due to various migration issues that are outside of my control. I'm afraid if we don't push hard for migration now it just won't happen for many packages.

Rexios80 avatar May 21 '24 14:05 Rexios80

Also how is this any different than regular deprecations in the Dart/Flutter SDKs. Those already immediately cause loss of points due to the analysis issues they generate.

Rexios80 avatar May 21 '24 15:05 Rexios80

@Rexios80: In my experience SDK deprecations rarely affect the score of a well-maintained package. wasm support is a new feature, less than a week old in a stable SDK. The effect and scale is not even comparable.

Again: if it is urgent for you for you, please consider to help out the developers of the packages that have a migration task ahead of them.

isoos avatar May 21 '24 15:05 isoos