[tools][web] Make Plugin Registrant file ephemeral. #102185

ditman · 2022-04-20T01:20:29Z

This change modifies flutter_tool slightly to make the web_plugin_registrant.dart file ephemeral (only created when needed, and not left in the users' working copy).

I needed to do some changes to the way a FlutterWeb Project works so it supports both modes of operation (on-disk for flutter build and in-memory for flutter run) so we can configure where the flutter_plugins.dart code creates the plugin registrar file for the web. Hence why it'd be nice that @blasten and @stuartmorgan took a quick look if possible :)

PluginMigrator

As suggested by @stuartmorgan, this change also adds a ProjectMigrator (class ScrubGeneratedPluginRegistrant) that deletes the old lib/generated_plugin_registrant.dart file if it exists, and cleans the .gitignore file by removing lines that are equal to: 'lib/generated_plugin_registrant.dart'.

The migrator runs when the user does flutter build web (only), with the following output:

$ flutter build web
Running "flutter pub get" in flutter_example...                  1,041ms

💪 Building with sound null safety 💪

generated_plugin_registrant.dart found. Deleted.
Upgrading .gitignore
Compiling lib/main.dart for the Web...                             8.5s

More changes (cleanup)

Creates a file_generators subdir, and moves there the file that generates flutter.js
- Unifies the code that generates the main entrypoint of a web app (and puts it in a file under file_generators)
- Extracts the code that generates the service worker (and puts it in a file under file_generators)
Prevents the service worker from prefetching assets/NOTICES. Issue.
Simplifies the web entrypoints a little bit (main.dart and web_plugin_registrar.dart), and ensures they won't trigger lint warnings. Issue.
Removes generated_plugin_registrar.dart from .gitignore template for new projects, now that it is created on-demand.

Issues

Fixes web: Service worker downloads NOTICES file directly at the start of the application #101828
Fixes [Proposal] Disable linting for generated_plugin_registrant.dart #99952
Somewhat opinates on Consider merging the web and desktop plugin registrant #80406 (doesn't merge the files)

Tests

WIP, this PR has been created in part to see where things burst at the seams :)

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

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

ditman · 2022-04-20T01:28:10Z

Null check operator used on a null value I probably forgot to check some entrypoint (or maybe tests are attempting to run some stuff for web without pre-configuring the FlutterProject earlier)

packages/flutter_tools/lib/src/flutter_plugins.dart

packages/flutter_tools/lib/src/web/compile.dart

packages/flutter_tools/lib/src/project.dart

packages/flutter_tools/lib/src/flutter_plugins.dart

ditman · 2022-04-23T00:12:11Z

~~I seem to recall that I have at least two tests that test for the non-existence of the web_plugin_registrant, but can't find them right now... They'll eventually explode in CI, I guess :)~~

They did explode, but more broke because of the unconditional write of the registrant, than its contents! (Fixed)

yjbanov · 2022-04-23T00:19:31Z

I am not too familiar with the nuances of plugin registration logic, so while this PR LGTM I recommend getting LGTMs from @stuartmorgan and/or @christopherfujino.

ditman · 2022-04-26T02:32:28Z

Some recent changes to address PR comments:

Added a ProjectMigrator to scrub generated_plugin_registrant.dart
Separated injectPlugins from injectBuildTimePluginFiles
- injectPlugins does not support webPlatform anymore.
- injectBuildTimePluginFiles has a signature very similar to injectPlugins, in case it can be reused for other platforms. It only supports webPlatform for now.
Create a noop web_plugin_registrar.dart when the app has no plugins, so the rest of the build process doesn't need to care about the project having plugins or not.

I hope I've addressed your comments @stuartmorgan, PTAL!

ditman · 2022-04-26T20:54:45Z

(Rebased after 944fcda)

ditman · 2022-05-04T02:40:12Z

PTAL @stuartmorgan? Thanks!

stuartmorgan-g

LGTM with comment nits, with the caveat that I'm not very familiar with how the web bootstrapping works.

stuartmorgan-g · 2022-05-09T19:52:41Z

packages/flutter_tools/lib/src/flutter_plugins.dart

Don't you mean not required at project generation time? If not, I'm not sure what the distinction between "compile time" and "build time" is.

Yeah, I think I'm going to use "project creation time" and "build time", I can't think of better names :/

I also specified a little bit better what the actual difference between the two methods is:

one writes the generated plugin registrants in locations configured by each FlutterProject subclass,

the other writes the generated plugin registrant (later) and within the temp directory of the build in progress (maybe to a real filesystem, or to an in-memory one)

packages/flutter_tools/lib/src/flutter_plugins.dart

Jasguerrero

LGTM

Fix imports.

Fix flutter build + flutter run scripts to use the new generation function, instead of their hardcoded versions.

Fixes #101828

(This shouldn't be needed, since the generated files live outside of the main app's lib directory, and are only created when doing flutter run/build, but it's good practice that generated files don't interfere with user code anyway.) Fixes #99952

The file is now ephemeral, and generated only as needed, so there's no need for this gitignore to exist anymore.

Prevent ensureReadyForPlatformSpecificTooling from calling injectPlugins for the web, since at this point on the build we don't have enough info. Flutter web commands (build, run) know when to call injectPlugins.

* Remove webPlatform from injectPlugins, to remove a foot gun from the execution path. * Introduce injectBuildTimePluginFiles with a similar signature to injectPlugins so web (and maybe others?) can inject the plugin files it needs, when needed. * Always inject a web plugin registrant, either a NOOP, or something that actually registers plugins, to decrease tooling complexity down the line (when generating a main entrypoint, for example) * Cleanup of other unneeded things (like the buildDir property on the WebProject, that is now an argument passed to injectBuildTimePluginFiles. * Test injectBuildTimePluginFiles.

…ritten unconditionally.

… and hook it up at the beginning of the compile process, where the file used to be generated.

ditman · 2022-05-12T00:49:26Z

(Applying the correct tag this time 😅)

…flutter#102185)

) The Flutter CLI removes the `app/lib/generated_plugin_registrant.dart` automatically from `.gitignore` because it does not exist anymore, see flutter/flutter#102185. ``` $ flutter build web Running "flutter pub get" in flutter_example... 1,041ms 💪 Building with sound null safety 💪 generated_plugin_registrant.dart found. Deleted. Upgrading .gitignore Compiling lib/main.dart for the Web... 8.5s ``` This PR removes the `lib/generated_plugin_registrant.dart` from `.gitignore` as the Flutter CLI does when building for web.

flutter/flutter#102185 https://github.com/flutter/flutter/blame/63a8298d4bfa00cfca4e70ba84096fc417fac8f4/packages/flutter_tools/templates/app_shared/.gitignore.tmpl

flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Apr 20, 2022

ditman requested a review from yjbanov April 20, 2022 01:20

ditman added the platform-web Web applications specifically label Apr 20, 2022

flutter-dashboard bot added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Apr 20, 2022

ditman marked this pull request as ready for review April 20, 2022 23:30

stuartmorgan-g requested changes Apr 21, 2022

View reviewed changes

ditman requested review from christopherfujino and stuartmorgan-g April 23, 2022 01:39

ditman requested review from Jasguerrero and removed request for christopherfujino May 9, 2022 19:44

stuartmorgan-g approved these changes May 9, 2022

View reviewed changes

Jasguerrero approved these changes May 10, 2022

View reviewed changes

ditman added 12 commits May 10, 2022 18:18

Move flutter_js.dart to file_generators subdir.

8b671fc

Fix imports.

Unify generation of web's main.dart.

ae32171

Fix flutter build + flutter run scripts to use the new generation function, instead of their hardcoded versions.

Split flutter_service_worker_js to its own file.

59d1247

Fixes #101828

Make web_plugin_registrant.dart ephemeral

adc0675

Remove generated_plugin_registrant from gitignore.

c70aa61

The file is now ephemeral, and generated only as needed, so there's no need for this gitignore to exist anymore.

Do not expect generated_plugin_registrant to exist anymore.

2673201

Do not call injectPlugins on web without a configured project.

0912925

Prevent ensureReadyForPlatformSpecificTooling from calling injectPlugins for the web, since at this point on the build we don't have enough info. Flutter web commands (build, run) know when to call injectPlugins.

Do not call injectPlugins for web without configuring it first.

3c1ae66

Fix expectations about generated files.

474f334

Do not expect that assets/NOTICES is prefetched

7f30f7c

Restore _NullaryFunction (required for some tests to pass)

bd7b604

ditman added 6 commits May 10, 2022 18:18

Fix web plugin registrant integration tests.

a87d286

Override the TemplateRenderer in tests now that the web template is w…

558342d

…ritten unconditionally.

Add ProjectMigrator to clean old refs to generated_plugin_registrant,…

a6430ce

… and hook it up at the beginning of the compile process, where the file used to be generated.

Small fix to docs post-rebase

82890ed

Improve docs in the injectPlugins/injectBuildTimePluginFiles directories

9457020

ditman added waiting for PR to land (fixed) A fix is in flight waiting for tree to go green and removed waiting for PR to land (fixed) A fix is in flight labels May 11, 2022

fluttergithubbot merged commit 1af8cc1 into flutter:master May 12, 2022

engine-flutter-autoroll mentioned this pull request May 12, 2022

Roll Flutter from 40627e9ed058 to b4ead55fb997 (3 revisions) flutter/packages#1902

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 12, 2022

1af8cc1 [tools][web] Make Plugin Registrant file ephemeral. (flutter/…

b3de7f3

…flutter#102185)

ditman deleted the tools-make-plugin-registrant-ephemeral branch May 12, 2022 04:38

engine-flutter-autoroll mentioned this pull request May 12, 2022

Roll Flutter from a9ac7fb03be1 to 8bec125aaf72 (38 revisions) flutter/plugins#5704

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 12, 2022

1af8cc1 [tools][web] Make Plugin Registrant file ephemeral. (flutter/…

9538692

…flutter#102185)

srawlins mentioned this pull request Jun 28, 2022

Flutter 3.1.0-9.0.pre dart-archive/dart-services#874

Closed

ditman mentioned this pull request Jul 1, 2022

[tool][web] Create an early web plugin_registrant for dartpad. #106921

Merged

8 tasks

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2022

1af8cc1 [tools][web] Make Plugin Registrant file ephemeral. (flutter/…

036c619

…flutter#102185)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 30, 2022

1af8cc1 [tools][web] Make Plugin Registrant file ephemeral. (flutter/…

7135a15

…flutter#102185)

nilsreichardt mentioned this pull request Mar 13, 2023

Remove app/lib/generated_plugin_registrant.dart from gitignore SharezoneApp/sharezone-app#506

Merged

[tools][web] Make Plugin Registrant file ephemeral. #102185

[tools][web] Make Plugin Registrant file ephemeral. #102185

Uh oh!

Conversation

ditman commented Apr 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PluginMigrator

More changes (cleanup)

Issues

Tests

Pre-launch Checklist

Uh oh!

ditman commented Apr 20, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ditman commented Apr 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yjbanov commented Apr 23, 2022

Uh oh!

ditman commented Apr 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ditman commented Apr 26, 2022

Uh oh!

ditman commented May 4, 2022

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g May 9, 2022

Choose a reason for hiding this comment

Uh oh!

ditman May 11, 2022

Choose a reason for hiding this comment

Uh oh!

ditman May 11, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Jasguerrero left a comment

Choose a reason for hiding this comment

Uh oh!

ditman commented May 12, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ditman commented Apr 20, 2022 •

edited

Loading

ditman commented Apr 23, 2022 •

edited

Loading

ditman commented Apr 26, 2022 •

edited

Loading