[flutter_tools] skip web renderer defines unless web target is picked #75160
Conversation
| @@ -805,7 +805,7 @@ abstract class FlutterCommand extends Command<void> { | |||
| /// | |||
| /// Throws a [ToolExit] if the current set of options is not compatible with | |||
| /// each other. | |||
| Future<BuildInfo> getBuildInfo({ BuildMode forcedBuildMode }) async { | |||
| Future<BuildInfo> getBuildInfo({ BuildMode forcedBuildMode, bool updateWebDefines = true }) async { | |||
There was a problem hiding this comment.
Passing along a bool for web mode seems a bit special purpose. Is there something that could be passed here instead that would convey the same thing, but which could also encode other platforms. Maybe TargetPlatform?
There was a problem hiding this comment.
Definitely. though in this case we can have multiple target platforms - I could pass a Set< TargetPlatform> . I think we should (in due time) move the update to the web dart defines into the web build targets and resident web runner, since that is very web specific.
There was a problem hiding this comment.
That would remove the need to handle any target platform logic here
There was a problem hiding this comment.
If the change to move the logic into the web-specific code is small, that does seem like a better place, but if that change will be higher-risk, then filing an issue and adding a TODO sgtm.
|
I tried to keep this change small to reduce the risk for a cherry pick. We do still have time to rethink how we're setting the web defines before the release. LMK @zanderso |
|
I think there isn't really any harm in setting this define for run in all cases, except the windows bug. If this is a short term fix, then as a long term fix maybe we just revert this after the base64 encoding change to dart-defines? |
…#75804) * Added a ReorderableListView.builder constructor (#74697) Added constructor parameters to the ReorderableList that were missing from ScrollView. Also replace a lot of doc comments with macro templates to reduce duplication. * [flutter_tools] handle further devtools NPE (#74764) * [flutter_tools] skip web renderer defines unless web target is picked (#75160) * Catch VM Service disappearance from run/attach handler code (#75298) * [flutter_tools] handle null package (#75336) * Return an empty FlutterViews list when the service disappears (#75301) * fix analyzer * Apply Engine Cherrypicks Co-authored-by: Darren Austin <darrenaustin@google.com> Co-authored-by: Jonah Williams <jonahwilliams@google.com> Co-authored-by: Jenn Magder <magder@google.com>
Fixes #75157
Don't pass web renderer dart-defines unless the device is a JavaScript device. TODO, where those tests at.