Allow skipping webOnlyInitializePlatform in Flutter for Web by jonahwilliams · Pull Request #40301 · flutter/flutter

jonahwilliams · 2019-09-12T00:36:54Z

Description

Allow configuring whether webOnlyInitializePlatform is invoked by the shell automatically with the flag (--no)--web-initialize-platform to flutter run and build. Also fixes an issue with flutter build not checking for plugins.

cc @jacob314 PTAL and let me know if this is sufficient for your usecase

Also fixes #40389

…latform

jacob314 · 2019-09-12T04:37:21Z

Works like a charm! This will make my job incrementally porting from dart:html much easier!

jacob314 · 2019-09-12T05:17:06Z

packages/flutter_tools/lib/src/build_runner/build_script.dart

@@ -383,7 +390,9 @@ import "${path.url.basename(buildStep.inputId.path)}" as entrypoint;

 Future<void> main() async {
  registerPlugins(webPluginRegistry);


doesn't matter for my use case but should this initialization line also be inside the if block?

I'll leave it out for now, plugins are still a WIP

jacob314 · 2019-09-12T05:19:15Z

packages/flutter_tools/lib/src/build_runner/build_script.dart


 Future<void> main() async {
-  await ui.webOnlyInitializePlatform();
+  if ($initializePlatform) {


doesn't really matter as it will be tree shaken anyway but you could write
${initializePlatform ? 'await ui.webOnlyInitializePlatform();' : ''}
to generate code with the initialize line omitted depending on the style you are going for with this generated code.

To simplify these case I'm going to leave it inline, I think we can revist later on

jacob314 · 2019-09-12T05:20:12Z

packages/flutter_tools/lib/src/build_runner/web_compilation_delegate.dart

      .childDirectory('.dart_tool')
      .createSync();
+    final FlutterProject flutterProject = FlutterProject.fromDirectory(projectDirectory);
+    final bool hasWebPlugins = findPlugins(flutterProject)


is this hasWebPlugins line an unrelated bug fix?

jacob314

lgtm once there is a test.

jonahwilliams · 2019-09-12T21:56:52Z

I'm going to change the command line flag name to be (--no)--web-initialize-platform

…itialize

cbenhagen · 2019-09-12T22:02:49Z

@jonahwilliams guess you can close #40389 with this.. Sorry for the noise.

jonahwilliams · 2019-09-12T22:04:41Z

@cbenhagen no problem!

jacob314 · 2019-09-12T22:32:06Z

packages/flutter_tools/lib/src/commands/build_web.dart

    usesTargetOption();
    usesPubOption();
    addBuildModeFlags();
+    argParser.addFlag('web-initialize-platform',


why prefix a flag to the web tool with web-? Seems less ambiguous in this case than the general case that this flag is only about initializing the web platform.

I want to keep the flag the same as what is provided as in run.dart. Since that is not web specific, I prefixed it.

jacob314 · 2019-09-12T22:33:13Z

packages/flutter_tools/test/general.shard/build_runner/build_script_test.dart

+
+    await builder.build(mockBuildStep);
+
+    verify(mockBuildStep.writeAsString(any, argThat(contains('if (false)')))).called(1);


nit: probably would be cleaner if your integration test could check for an
if (false) { initalizeWebOnlyPlatform(); }

jacob314

with a couple small nits. Thanks for implementing this so quickly!

codecov · 2019-09-13T02:18:55Z

Codecov Report

Merging #40301 into master will increase coverage by 0.66%.
The diff coverage is 75%.

@@            Coverage Diff             @@
##           master   #40301      +/-   ##
==========================================
+ Coverage   58.56%   59.22%   +0.66%     
==========================================
  Files         192      193       +1     
  Lines       18595    18712     +117     
==========================================
+ Hits        10890    11083     +193     
+ Misses       7705     7629      -76

Flag	Coverage Δ
#flutter_tool	`59.22% <75%> (+0.66%)`	⬆️

Impacted Files	Coverage Δ
packages/flutter_tools/lib/src/test/runner.dart	`0% <ø> (ø)`	⬆️
...lib/src/build_runner/web_compilation_delegate.dart	`24.24% <0%> (-1.16%)`	⬇️
packages/flutter_tools/lib/src/web/compile.dart	`84.37% <100%> (ø)`	⬆️
...ges/flutter_tools/lib/src/build_runner/web_fs.dart	`23.42% <100%> (+1.2%)`	⬆️
packages/flutter_tools/lib/src/commands/run.dart	`64.56% <100%> (+1.01%)`	⬆️
...ools/lib/src/build_runner/resident_web_runner.dart	`78.52% <100%> (+0.14%)`	⬆️
packages/flutter_tools/lib/src/device.dart	`61.76% <100%> (ø)`	⬆️
...ages/flutter_tools/lib/src/commands/build_web.dart	`73.33% <50%> (+1.9%)`	⬆️
...utter_tools/lib/src/build_runner/build_script.dart	`20% <66.66%> (ø)`
...ackages/flutter_tools/lib/src/commands/doctor.dart	`57.14% <0%> (-7.15%)`	⬇️
... and 22 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 576a455...dc00651. Read the comment docs.

…40301)

allow not calling webOnlyInitializePlatform with (--no)--initialize-p…

6bd23a0

…latform

fluttergithubbot added the tool Affects the "flutter" command-line tool. See also t: labels. label Sep 12, 2019

googlebot added the cla: yes label Sep 12, 2019

jonahwilliams changed the title ~~allow not calling webOnlyInitializePlatform with (--no)--initialize-p…~~ Allow skipping webOnlyInitializePlatform in Flutter for Web Sep 12, 2019

jonahwilliams added the work in progress; do not review label Sep 12, 2019

jonahwilliams changed the title ~~Allow skipping webOnlyInitializePlatform in Flutter for Web~~ [WIP] Allow skipping webOnlyInitializePlatform in Flutter for Web Sep 12, 2019

jacob314 reviewed Sep 12, 2019

View reviewed changes

update tests for both cases

5471694

jonahwilliams changed the title ~~[WIP] Allow skipping webOnlyInitializePlatform in Flutter for Web~~ Allow skipping webOnlyInitializePlatform in Flutter for Web Sep 12, 2019

jonahwilliams removed the work in progress; do not review label Sep 12, 2019

jonahwilliams requested review from harryterkelsen, jacob314 and yjbanov September 12, 2019 21:53

Merge branch 'master' of github.com:flutter/flutter into secret_no_in…

e2bb2a9

…itialize

cbenhagen mentioned this pull request Sep 12, 2019

[web] Release build is not registering web plugins #40389

Closed

jacob314 reviewed Sep 12, 2019

View reviewed changes

jonahwilliams added 2 commits September 12, 2019 15:49

fix test case and address comments

84dba1e

Merge branch 'master' into secret_no_initialize

ad431a0

harryterkelsen approved these changes Sep 12, 2019

View reviewed changes

Update run.dart

3f1a3f6

Update build_web.dart

dc00651

jonahwilliams merged commit 69a2964 into flutter:master Sep 13, 2019

jonahwilliams deleted the secret_no_initialize branch September 13, 2019 17:20

Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Sep 30, 2019

Allow skipping webOnlyInitializePlatform in Flutter for Web (flutter#…

627fc22

…40301)

github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2021

		@@ -383,7 +390,9 @@ import "${path.url.basename(buildStep.inputId.path)}" as entrypoint;

		Future<void> main() async {
		registerPlugins(webPluginRegistry);


		await builder.build(mockBuildStep);

		verify(mockBuildStep.writeAsString(any, argThat(contains('if (false)')))).called(1);

Conversation

jonahwilliams commented Sep 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

jacob314 commented Sep 12, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jacob314 left a comment

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented Sep 12, 2019

Uh oh!

cbenhagen commented Sep 12, 2019

Uh oh!

jonahwilliams commented Sep 12, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jacob314 left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Sep 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jonahwilliams commented Sep 12, 2019 •

edited

Loading

codecov bot commented Sep 13, 2019 •

edited

Loading