Skip to content
This repository was archived by the owner on Dec 6, 2023. It is now read-only.

Flutter 3.1.0-9.0.pre #874

Closed
wants to merge 17 commits into from

Conversation

domesticmouse
Copy link
Contributor

No description provided.

@domesticmouse domesticmouse requested a review from RedBrogdon June 16, 2022 13:17
@domesticmouse domesticmouse marked this pull request as draft June 22, 2022 02:46
@domesticmouse domesticmouse requested a review from srawlins June 27, 2022 04:12
@domesticmouse
Copy link
Contributor Author

Hey @srawlins, can you give guidance on what the Web bootstrap should now be? Flutter 3.1.0-9.0.pre has broken the old bootstrap file that expected generated_plugin_registrant.dart and I'm unsure what the bootstrap should now look like...

@srawlins
Copy link
Contributor

Do you have CLs where they've been changing what the bootstrap file is going to be generated as? The CI failures don't look super helpful, but can we get the server logs for those requests?

@domesticmouse domesticmouse marked this pull request as ready for review June 28, 2022 01:53
@domesticmouse
Copy link
Contributor Author

There are no server logs as this hasn't been landed yet. Once this PR lands, the beta server will automatically adopt it, and start failing requests that rely on plugins. I'm guessing that means most of the Firebase packages, for starters.

The before and after on the build method looks like this:

The delta between the two is too large to get GitHub to generate a visual diff.

@srawlins
Copy link
Contributor

For the server logs, I just mean the server during the test. "Expected 200, Actual 400" isn't super helpful. So we'd have to add debugging or something to see why it's so angry.

@domesticmouse
Copy link
Contributor Author

With just the version of the Dart SDK bumped to 3.1.0-9.0.pre and feeding the DDC compiler the following:

import 'package:flutter/material.dart';

void main() {
  for (int i = 0; i < 4; i++) {
    print('hello ${i}');
  }
}

We see the following error:

Error: Error when reading '/var/folders/5y/4dz9m6091ms_72g3zk9pvjn4003nrc/T/dartpadU0yzQJ/.dart_tool/package_config.json': No such file or directory
Error: Couldn't resolve the package 'flutter_web_plugins' in 'package:flutter_web_plugins/flutter_web_plugins.dart'.
/var/folders/5y/4dz9m6091ms_72g3zk9pvjn4003nrc/T/dartpadU0yzQJ/lib/bootstrap.dart:2:8: Error: Not found: 'package:flutter_web_plugins/flutter_web_plugins.dart'
import 'package:flutter_web_plugins/flutter_web_plugins.dart';
       ^
/var/folders/5y/4dz9m6091ms_72g3zk9pvjn4003nrc/T/dartpadU0yzQJ/lib/bootstrap.dart:3:8: Error: Error when reading '/var/folders/5y/4dz9m6091ms_72g3zk9pvjn4003nrc/T/dartpadU0yzQJ/lib/generated_plugin_registrant.dart': No such file or directory
import 'generated_plugin_registrant.dart';
       ^
/var/folders/5y/4dz9m6091ms_72g3zk9pvjn4003nrc/T/dartpadU0yzQJ/lib/bootstrap.dart:7:19: Error: Undefined name 'webPluginRegistrar'.
  registerPlugins(webPluginRegistrar);
                  ^^^^^^^^^^^^^^^^^^
/var/folders/5y/4dz9m6091ms_72g3zk9pvjn4003nrc/T/dartpadU0yzQJ/lib/bootstrap.dart:7:3: Error: Method not found: 'registerPlugins'.
  registerPlugins(webPluginRegistrar);
  ^^^^^^^^^^^^^^^

With the contents of this PR, the returned error is now:

/var/folders/5y/4dz9m6091ms_72g3zk9pvjn4003nrc/T/dartpadHQdH4t/lib/bootstrap.dart:5:8: Error: Error when reading '/var/folders/5y/4dz9m6091ms_72g3zk9pvjn4003nrc/T/dartpadHQdH4t/lib/web_plugin_registrant.dart': No such file or directory
import 'web_plugin_registrant.dart' as pluginRegistrant;
       ^
/var/folders/5y/4dz9m6091ms_72g3zk9pvjn4003nrc/T/dartpadHQdH4t/lib/bootstrap.dart:19:24: Error: Method not found: 'registerPlugins'.
      pluginRegistrant.registerPlugins();
                       ^^^^^^^^^^^^^^^

@srawlins
Copy link
Contributor

Ah that's super helpful.

Well I think the command to git diff that file between 3.1.0 and 3.1.0-9.0.pre (are those correct? We're currently running 3.1.0 in the beta branch, and we're moving to 3.1.0-9.0.pre?) is git diff 3.1.0..3.1.0-9.0.pre packages/flutter_tools/lib/src/build_system/targets/web.dart which I think amounts to this commit from May 11 (but flutter/flutter#106584 from 4 days ago may also be relevant). Is it weird that it might be from May 11? Is that too early? One important change in there seems to be the new main_dart.dart file.

The relevant bits that I remember, ragarding how we generate a plugin registrant file are:

@srawlins
Copy link
Contributor

Maybe @ditman can help out here. I'm not sure exactly what changed with flutter/flutter#102185, but our current process for running a Flutter Web app is:

  1. User clicks 'run' and the browser sends the user script to the server.
  2. We create a little temp project directory with the user script as lib/main.dart, I think a web/index.html maybe, and a pubspec with entries that depend on what the user script contained ("Oh it looks like they need firebase, I'll add that").
  3. We run flutter packages get in that dir, here.
  4. We expect lib/generated_plugin_registrant.dart has been created.
  5. We create our own lib/bootstrap.dart file which refers to that lib/generated_plugin_registrant.dart file here.
  6. We compile the user's app by directly calling dartdevc (DDC), here.

I suspect that we create our own bootstrap.dart file because we don't run flutter build. I don't think I know why we don't run flutter build. Something about DDC? require.js? But there are ways to run flutter build which use DDC.

So I imagine flutter build is creating one or more files that we would now need to generate ourselves.

@ditman
Copy link

ditman commented Jun 28, 2022

  1. We expect lib/generated_plugin_registrant.dart has been created.

Ahhh, that is no longer true, the file gets created by flutter build / flutter run at the moment it's needed to compile the app!


There was also a small change in the contents of the plugin registrant and how it's generated that might explain the errors that @domesticmouse is seeing:

So when there are no plugins we generate a file that has just a noop registerPlugins method:

And when there ARE plugins, we assume we can fallback to the webPluginRegistrar, but the default entrypoint always calls this as registerPlugins() now (so we save on a dependency on flutter_web_plugins on the bootstrap code itself):

This is the new bootstrap code:

Note that the bootstrap just defines the runApp and registerPlugins methods, which then get called by the programmer (if they choose to use the new initialization, from their JavaScript, example) or by the engine "automatically", as it did before this change (so you wouldn't need any changes to your old index.html if it's being generated somewhere). Here's the source for webOnlyWarmupEngine, for reference.


So I guess, your flutter "bootstrap.dart" here needs to be rewritten around ui.webOnlyWarmupEngine.

The plugin registrant seems harder, I guess you guys don't want to flutter build because it slows down execution compared to DDC?

@domesticmouse
Copy link
Contributor Author

The plugin registrant seems harder, I guess you guys don't want to flutter build because it slows down execution compared to DDC?

My understanding is that with DDC we can do a chunk of work upfront (e.g. compiling Flutter and Dart SDK to JS) and then have a quick per-request compilation. We want to be able to compile flutter code in sub second timeframes to prevent blowing out the number of serving machines we need. It's also nice for users to have as quick as possible response to the compile-and-run button.

Moving from DDC to full compilation would be a major undertaking, invalidating several pieces of our infrastructure, and unlikely to be doable before the next stable release.

@ditman
Copy link

ditman commented Jun 28, 2022

So, we should restore the generated_plugin_registrant.dart on flutter pub get?

We could put it somewhere in .dart_tool for you to move around as needed? The goal of the change above was to not create it the users' working copy (inside lib)

@ditman
Copy link

ditman commented Jun 28, 2022

Created this issue: flutter/flutter#106765 /cc @yjbanov

@domesticmouse
Copy link
Contributor Author

So, we should restore the generated_plugin_registrant.dart on flutter pub get?

We could put it somewhere in .dart_tool for you to move around as needed? The goal of the change above was to not create it the users' working copy (inside lib)

This works for me!

@domesticmouse domesticmouse marked this pull request as draft July 4, 2022 23:34
@ditman
Copy link

ditman commented Jul 6, 2022

A fix for the generation of the web plugin_registrant on pub get has landed on master, and a cherry-pick for it has been requested for beta.

@domesticmouse
Copy link
Contributor Author

A fix for the generation of the web plugin_registrant on pub get has landed on master, and a cherry-pick for it has been requested for beta.

I'll close this PR, and create a new PR once the new beta release is rolled.

@ditman
Copy link

ditman commented Jul 21, 2022

@domesticmouse the generation of the flutter plugin registrant on flutter pub get has landed in flutter beta: 3.3.0-0.0.pre.

Apologies for the delay, my CP request was skipped in favor of the normal beta flow.

@domesticmouse domesticmouse deleted the domesticmouse-flutter-3.1.0-9.0.pre branch July 26, 2022 02:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants