Skip to content

no longer suggest package imports for dart:html (and related) packages #54610

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

Closed
devoncarew opened this issue Jan 12, 2024 · 8 comments
Closed
Assignees
Labels
devexp-assist Issues with analysis server assists devexp-completion Issues with the analysis server's code completion feature legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on

Comments

@devoncarew
Copy link
Member

devoncarew commented Jan 12, 2024

For quick fixes and code completion, we suggest classes from dart:html when that library isn't imported, and offer quick fixes to add an import when there's an unresolved class. We should no longer suggest from the dart:html library (unless it's already imported). We're going to start recommending that people use package:web for new projects, and these existing quick fixes will make the migration story less clear.

See https://api.dart.dev/main/95cad9c8a6911dc8feb93f0ee75acf52ea959bfe/index.html for the 'legacy' web libraries:

  • dart:html
  • dart:indexed_db
  • dart:js
  • dart:js_util
  • dart:svg
  • dart:web_audio
  • dart:web_gl

If people are already using them that's fine, we just don't want to suggest adding an import for them.

Related discussion on dart-lang/web#141 (comment).

cc @kevmoo

@devoncarew devoncarew added the legacy-area-analyzer Use area-devexp instead. label Jan 12, 2024
@pq pq added devexp-completion Issues with the analysis server's code completion feature devexp-assist Issues with analysis server assists P2 A bug or feature request we're likely to work on labels Jan 16, 2024
@scheglov scheglov self-assigned this Jan 23, 2024
@scheglov
Copy link
Contributor

@bwilkerson
Copy link
Member

We're going to start recommending ...

🎉

What's the time frame for this? We have users that are using the nightly builds, and I want to make sure that the alternative is available before we stop suggesting the current solution.

I know that there's additional tooling work that we are hoping to do (minimally some lints), but I don't know whether we want to try to finish that work before the proposed completion change.

@parlough
Copy link
Member

parlough commented Jan 23, 2024

What's the time frame for this?

\cc @kevmoo

I want to make sure that the alternative is available before we stop suggesting the current solution...

The current replacements are all available in some form:


I am curious is the suggested change if a library doesn't import it or a package doesn't? It could cause some UX regressions if per-library. Some app and package developers may not have a chance to migrate yet or might choose to stay on a Dart version with dart:html and not surfacing suggestions could be quite painful. The SDK constraint should likely be considered as well. Personally, I still prefer a user-controllable toggle of some sort as an intermediate step (#54347) towards this point.

@devoncarew
Copy link
Member Author

The doc changes for this recommendation have already landed (eg. see https://api.dart.dev/main), and package:web is already something people can use (dartpad has migrated, flutter, ...).

Note that the intention here isn't to prevent people from using dart:html, but that we don't re-add imports to it when they're in the process of migrating to package:web.

cc @kevmoo wrt timing and messaging

@kevmoo
Copy link
Member

kevmoo commented Jan 23, 2024

We should be careful pushing the ENTIRE ecosystem here. Like we don't want warning showing up in Google3, etc.

I think this is okay for Dart 3.4. We're just talking about suggestions, right?

@devoncarew
Copy link
Member Author

Yes, just suggestions to add the import; I think the comment here illustrates the problem best: dart-lang/web#141 (comment)

@bwilkerson
Copy link
Member

We're just talking about suggestions, right?

That's my understanding.

I think this is okay for Dart 3.4.

But is it ok for people on the nightly build to get it tomorrow, or for people on the dev release channel to get it next week?

@kevmoo
Copy link
Member

kevmoo commented Jan 23, 2024

But is it ok for people on the nightly build to get it tomorrow, or for people on the dev release channel to get it next week?

Yep. See if we get any reports. A good canary in the coal mine.

copybara-service bot pushed a commit that referenced this issue Jan 24, 2024
Bug: #54610
Change-Id: Ibf01a25ac6e67fb99370ed4d6119c9f8e92d1f5a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/348081
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devexp-assist Issues with analysis server assists devexp-completion Issues with the analysis server's code completion feature legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on
Projects
None yet
Development

No branches or pull requests

6 participants