Move package:web dependency to dev dependency #139696
Conversation
Pinning the package:web dependency constrains downstream packages from using newer versions and making sure they support the version pinned in Flutter. Since the usage of package:web in Flutter is light, we should instead have a small shim like the engine and keep package:web as a dev dependency only.
github-actions
bot
added
a: tests
|
There's an open question on where |
| @@ -421,8 +421,8 @@ class HtmlElementView extends StatelessWidget { | |||
| /// used by the app to customize the element by adding attributes and styles. | |||
| /// | |||
| /// ```dart | |||
| /// import 'package:flutter/src/web.dart' as web; | |||
There was a problem hiding this comment.
Not sure if this is okay :)
There was a problem hiding this comment.
What was wrong with the old code?
There was a problem hiding this comment.
Dartdoc/analysis would attempt to run this code, but because package:web is no longer a dependency, it would not resolve.
There was a problem hiding this comment.
Sorry for derailing the PR, but I'm starting to have second thoughts about it:
This example is meant to work when developers copy it into their apps, but it looks like it won't? This might be an indicator that removing the dependency on package:web may not be a good idea. If users are expected to use the core framework APIs along with package:web then for all intents and purposes the framework already depends on it despite the hacks we use to sever the dependency for pub get's sake.
Maybe we should pay the cost of pinning so our developers do not pay the cost of poor ergonomics. In fact, now that I think about this class, it is a web-specific class but it lives in the platform-agnostic widgets/platform_view.dart library. We should move it into its own widgets/platform_view_web.dart library and teach it to speak in terms of package:web interfaces instead of Object. That library will be imported behind a conditionally imported library that has access to web-specific libraries, including package:web.
There was a problem hiding this comment.
For EXAMPLES, folks SHOULD use pkg:web.
Casting works fine. Types are erased.
@srujzs is just working around tooling silly here.
That's the beauty here. Let's lean into it!
There was a problem hiding this comment.
It should still work. This is an analysis issue that is specific to the doc comment. Users should still import package:web/web.dart and the code will still work fine even with the shim we introduce here. @staticInterop (and later, extension types on JSObject) can be casted freely from one type to another unlike dart:html types, because we only ever check that the type is a JSObject. I don't want people to be able to use the shim besides for the purpose in this CL.
If we want to treat this comment as a guide for users (which I agree we should), then it's a matter of ignoring that analysis issue.
There was a problem hiding this comment.
How about we move the example to flutter.dev and remove it from the docs?
There was a problem hiding this comment.
SGTM! At a minimum just open the issue. This can go in our embedding docs
There was a problem hiding this comment.
I've removed the example from this PR and filed https://github.com/flutter/flutter/issues/140010.
packages/flutter/lib/src/web.dart
Outdated
| /// Flutter as a dependency. | ||
| /// | ||
| /// This should stay in sync with `package:web` as much as possible to make it | ||
| /// easier to add new members as needed. |
There was a problem hiding this comment.
What is the process for keeping it in sync with package:web? Is this file generated? If so, we should document the process for regenerating it. If it's manually written, we should document the guidelines for maintaining it.
There was a problem hiding this comment.
It is indeed manually copied over. I've added a few comments guiding users on how to maintain and extend it, let me know if there needs to be more, thanks!
| @@ -421,8 +421,8 @@ class HtmlElementView extends StatelessWidget { | |||
| /// used by the app to customize the element by adding attributes and styles. | |||
| /// | |||
| /// ```dart | |||
| /// import 'package:flutter/src/web.dart' as web; | |||
There was a problem hiding this comment.
What was wrong with the old code?
|
Friendly ping on this. |
FYI - I've ignored these lints for the file for now and documented why. |
|
auto label is removed for flutter/flutter/139696, due to Pull request flutter/flutter/139696 is not in a mergeable state. |
auto-submit
bot
removed
the
autosubmit
github-actions
bot
added
the
tool
…e-packages --force-upgrade A future roll should do this and not this PR.
github-actions
bot
removed
the
tool
flutter/flutter@a51e33a...2407f69 2023-12-15 58529443+srujzs@users.noreply.github.com Move package:web dependency to dev dependency (flutter/flutter#139696) 2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9524a185b055 to 986a6fe198dc (1 revision) (flutter/flutter#140221) 2023-12-15 engine-flutter-autoroll@skia.org Roll Packages from 1151191 to 3f2e16b (9 revisions) (flutter/flutter#140218) 2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7a50221733c2 to 9524a185b055 (1 revision) (flutter/flutter#140217) 2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from 767223f7a4f8 to 7a50221733c2 (1 revision) (flutter/flutter#140216) 2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from 91f65eea0c11 to 767223f7a4f8 (1 revision) (flutter/flutter#140210) 2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from a47da28c9a62 to 91f65eea0c11 (1 revision) (flutter/flutter#140207) 2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from cde1a596432d to a47da28c9a62 (1 revision) (flutter/flutter#140204) 2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from 46ff5c08a905 to cde1a596432d (1 revision) (flutter/flutter#140200) 2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from a17bb0a63b7e to 46ff5c08a905 (1 revision) (flutter/flutter#140198) 2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from 4cb3ba7a85f6 to a17bb0a63b7e (1 revision) (flutter/flutter#140196) 2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from 0e7248d43251 to 4cb3ba7a85f6 (14 revisions) (flutter/flutter#140195) 2023-12-15 polinach@google.com Increase versions of leak tracker libraries. (flutter/flutter#140018) 2023-12-15 magder@google.com Set compile test iOS app target version to not embed Swift runtime (flutter/flutter#140188) 2023-12-15 58190796+MitchellGoodwin@users.noreply.github.com Cupertino text clear label (flutter/flutter#129727) 2023-12-15 xilaizhang@google.com [github actions] use token from real user flutter mirror bot (flutter/flutter#140191) 2023-12-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Flutter Engine from 0e7248d43251 to 0b0fab821536 (4 revisions)" (flutter/flutter#140194) 2023-12-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 0e7248d43251 to 0b0fab821536 (4 revisions) (flutter/flutter#140180) 2023-12-14 lsaudon@gmail.com feat: Add onTapAlwaysCalled in TextFormField (flutter/flutter#140089) 2023-12-14 49699333+dependabot[bot]@users.noreply.github.com Bump actions/upload-artifact from 3.1.3 to 4.0.0 (flutter/flutter#140177) 2023-12-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 2140942444ea to 0e7248d43251 (2 revisions) (flutter/flutter#140176) 2023-12-14 ybz975218925@gmail.com fix reorderable_list drop animation (flutter/flutter#139362) 2023-12-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 997d3dfa1e74 to 2140942444ea (4 revisions) (flutter/flutter#140171) 2023-12-14 38147403+sharmashashi@users.noreply.github.com Fix BottomNavigationBarItem label overflow (flutter/flutter#120206) 2023-12-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from a565cea256c7 to 997d3dfa1e74 (2 revisions) (flutter/flutter#140170) 2023-12-14 chingjun@google.com Revert "Dynamic view sizing" (flutter/flutter#140165) 2023-12-14 aki.nishi.work@gmail.com �: fix cupertionActionSheet design (flutter/flutter#134345) 2023-12-14 104349824+huycozy@users.noreply.github.com Make improvements to existing new issue templates (flutter/flutter#140142) 2023-12-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from caf33276468b to a565cea256c7 (1 revision) (flutter/flutter#140163) 2023-12-14 parlough@gmail.com Expand and update a few release.yml categories (flutter/flutter#140120) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC dit@google.com,rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
XilaiZhang
added
cp: beta
|
Unable to create the revert pull request due to ProcessException: Standard out |
auto-submit
bot
removed
the
revert
|
FYI this broke Google testing. Please take a look at b/316639209 |
This reverts commit 2407f69.
Pinning the package:web dependency constrains downstream packages from using newer versions and making sure they support the version pinned in Flutter. Since the usage of package:web in Flutter is light, we should instead have a small shim like the engine and keep package:web as a dev dependency only.
Pinning the package:web dependency constrains downstream packages from using newer versions and making sure they support the version pinned in Flutter. Since the usage of package:web in Flutter is light, we should instead have a small shim like the engine and keep package:web as a dev dependency only.
Pinning the package:web dependency constrains downstream packages from using newer versions and making sure they support the version pinned in Flutter. Since the usage of package:web in Flutter is light, we should instead have a small shim like the engine and keep package:web as a dev dependency only.
Pre-launch Checklist
///).