Skip to content

Conversation

@ditman
Copy link
Member

@ditman ditman commented Sep 26, 2024

This PR tweaks wrapWithDefaultView (used by runApp) to raise a StateError with a legible error message when the platformDispatcher.implicitView is missing (for example, when enabling multi-view embedding on the web), instead of crashing with an unexpected nullCheck a few lines below.

  • Before:
    Screenshot 2024-09-25 at 7 33 47 PM

  • After:
    Screenshot 2024-09-26 at 5 01 49 PM

Issues

Tests

Added a test to ensure the assertion is thrown when the implicitView is missing. Had to hack a little because I couldn't find any clean way of overriding the implicitView. The problem is that the flutter_test bindings use runApp internally a couple of times, so I can only disable the implicitView inside the test body (and must re-enable it before returning). Not sure if it's the best way, but it seems to do the trick for this simple test case!

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

This allows us to render a more user-friendly error, instead of failing
with an "unexpected null value" a few lines below.
@github-actions github-actions bot added the framework flutter/packages/flutter repository. See also f: labels. label Sep 26, 2024
Widget wrapWithDefaultView(Widget rootWidget) {
assert(
platformDispatcher.implicitView != null,
'The implicitView is null. Cannot create a default View from `runApp`. '
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error uses jargon that most Flutter developers won't know. This might frustrate users.

Perhaps we should add more context? Maybe something like:

runApp requires an implicit view.

runApp renders the widget tree into a view provided by the platform,
called the "implicit view". However, the platform did not provide an
implicit view.

Try using runWidget instead of runApp.

See: https://flutter.dev/to/web-runwidget

Copy link
Contributor

@dkwingsmt dkwingsmt Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it's a public API, I think we should not assume it 100% comes from runApp. Maybe

The app requested an implicit view, but the platform doesn't have one.

This is likely because the app called runApp to start up, which expects the platform to provide a default view to render into ("the implicit view"), while the engine has multi-view mode enabled, which requires the app handle view creation.

Try using runWidget instead of runApp. See also https://flutter.dev/to/web-runwidget

Copy link
Member Author

@ditman ditman Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@loic-sharma @dkwingsmt I ended up with something like this, I hope it makes more sense!

The app is attempting to render into a default View that isn't available (platformDispatcher.implicitView == null).
This is likely because the platform has multi-view mode enabled, but the app is calling runApp to render its root Widget.
Try using runWidget instead of runApp to start your app. runWidget allows you to provide a View widget, without requiring a default on the platform.
See: https://flutter.dev/to/web-multiview-runwidget

(Tried to use lingo closer to that in the wrapWithDefaultView method docs.)

Copy link
Member

@loic-sharma loic-sharma Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I slightly prefer Tong's wording.

Some context:

  1. On desktop, the north star is that the app will call runApp even though there's no implicit view. runApp will know how to create a window to host the widget tree.
  2. On desktop, you can have multiple views while also having an implicit view. Thus, on desktop you can call runApp even if you have multi-view mode enabled

Since it's a public API, I think we should not assume it 100% comes from runApp.

Should we move this error to runApp then and then make wrapWithDefaultView assert that platformDispatcher.implicitView != null?

What do y'all think about this? I tried to merge bits from all our messages :)

The app requested an implicit view, but the platform did not provide one.

runApp expects the platform to provide a default view to render into ("the implicit view").
However, the platform did not provide a default "implicit" view, likely because it has multi-view mode enabled.

Try using runWidget instead of runApp to start your app. runWidget allows you to provide a View widget, without requiring a default view.
See: https://flutter.dev/to/web-multiview-runwidget

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

I think checking it in wrapWithDefaultView is better than runApp since wrapWithDefaultView is also a public API and users can still call it incorrectly (which is a user error and you shouldn't assert on).

Copy link
Member Author

@ditman ditman Sep 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I applied @loic-sharma's suggestion! If @dkwingsmt is happy, I'm happy too, let's autosubmit this :P


(Off-topic)

runApp will know how to create a window to host the widget tree.
...
you can have multiple views while also having an implicit view

We need to discuss this elsewhere, I'm interested :P

Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with nits.

I'm not familiar with multiview so I'm happy to defer to anyone else that might be, but I know @goderbauer is out for awhile. This looks good from my perspective though. Using throwsA to check the assertion seems fine.

Also it looks like there are failures.

ditman added a commit to flutter/website that referenced this pull request Sep 26, 2024
We're adding a more descriptive error when a misconfigured multi-view web app attempts to run (flutter/flutter#155734), and we want a permalink to the relevant bit of the docs.

This PR adds a `/to/web-multiview-runwidget` redirect, pointing to the correct part of the docs.
sfshaza2 pushed a commit to flutter/website that referenced this pull request Sep 26, 2024
## Description

We're adding a more descriptive error when a misconfigured multi-view
web app attempts to run (flutter/flutter#155734), and we want a
permalink to the relevant bit of the docs.

This PR adds a `/to/web-multiview-runwidget` redirect, pointing to the
correct part of the docs.

## Issues

Needed for flutter/flutter#155734

## Presubmit checklist

- [ ] This PR is marked as draft with an explanation if not meant to
land until a future stable release.
- [x] This PR doesn’t contain automatically generated corrections
(Grammarly or similar).
- [x] This PR follows the [Google Developer Documentation Style
Guidelines](https://developers.google.com/style) — for example, it
doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person).
- [ ] This PR uses [semantic line
breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks)
of 80 characters or fewer.
@ditman ditman changed the title Assert implicitView != null on wrapWithDefaultView. Throw StateError when implicitView is null on wrapWithDefaultView. Sep 26, 2024
Copy link
Member

@loic-sharma loic-sharma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error message looks good to me, thanks!

Copy link
Contributor

@dkwingsmt dkwingsmt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dkwingsmt dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 27, 2024
@auto-submit auto-submit bot merged commit f9a76ae into flutter:master Sep 27, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 27, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 27, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 27, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 28, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 28, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 1, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 1, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 1, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 1, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 1, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 2, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 2, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 2, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 2, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 2, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 2, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 4, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 4, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 4, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 5, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 5, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 6, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 6, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 7, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Oct 7, 2024
Manual roll requested by [email protected]

flutter/flutter@fa402c8...ead6b0d

2024-09-27 [email protected] Remove left-over traces of "link-dry-run" - which isn't used anywhere in flutter (flutter/flutter#155820)
2024-09-27 [email protected] Roll Flutter Engine from e57b440ec4ee to 7c603de2dca7 (5 revisions) (flutter/flutter#155811)
2024-09-27 [email protected] Fix DropdownMenu rendered behind AppBar (flutter/flutter#155539)
2024-09-27 [email protected] Roll Flutter Engine from 53517772a5b0 to e57b440ec4ee (8 revisions) (flutter/flutter#155799)
2024-09-27 [email protected] Throw StateError when implicitView is null on `wrapWithDefaultView`. (flutter/flutter#155734)
2024-09-27 [email protected] Roll packages manually (flutter/flutter#155786)
2024-09-27 [email protected] fix: SelectableText should handle focus changes (flutter/flutter#155771)
2024-09-27 [email protected] Use flutter from in same repo (not path) in `generate_gradle_lockfiles.dart` (again) (flutter/flutter#155794)
2024-09-26 [email protected] Use flutter from in same repo (not path) in `generate_gradle_lockfiles.dart` (flutter/flutter#155790)
2024-09-26 [email protected] `RenderParagraph` should invalidate its `_SelectableFragment`s cached rects on window size updates (flutter/flutter#155719)
2024-09-26 [email protected] Fix broken text field with set hint and min and max lines(#153183) (flutter/flutter#153235)
2024-09-26 [email protected] Roll Flutter Engine from 9e6133e8d906 to 53517772a5b0 (1 revision) (flutter/flutter#155772)
2024-09-26 [email protected] Fix line-wrapping in `flutter create` error message. (flutter/flutter#150325)
2024-09-26 [email protected] remove fujino from CODEOWNERS (flutter/flutter#155369)

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 [email protected],[email protected] 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
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unexpected null value error appears when running multiView on Flutter Web

4 participants