Skip to content

Conversation

@eyebrowsoffire
Copy link
Contributor

This fixes #7444

  • Moves bootstrapping logic into templated flutter_bootstrap.js file.
  • Changes to FlutterLoader.load() API from FlutterLoader.loadEntrypoint() API.

@eyebrowsoffire eyebrowsoffire requested a review from a team as a code owner April 16, 2024 20:08
@eyebrowsoffire eyebrowsoffire requested review from CoderDake and removed request for a team April 16, 2024 20:08
function bootstrapAppFor1P() {
_flutter.loader.load({
config: {
canvasKitBaseUrl: 'canvaskit/'
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this is a change from the previous implementation, where we did not set the canvasKitBaseUrl field for 1P. @elliette do you remember why we needed separate bootstrapping for 1P? And does this look okay to you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that's just an oversight by me. I can remove that.

Copy link
Member

Choose a reason for hiding this comment

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

I forget why the canvasKitBasUrl is only required for 3P, but in general the assets are located in slightly different locations for 1P vs 3P which is why there are two different implementations.

@kenzieschmoll kenzieschmoll requested a review from elliette April 16, 2024 20:25

// Bootstrap app for 1P environments:
function bootstrapAppFor1P() {
_flutter.loader.load();
Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing we also need to specify the entrypointUrl as main.dart.js, see #5705

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should not need that, and the new API does not take an entryPointUrl as a parameter. The old API should have had that as the default and wouldn't have needed that anyway, so I'm not sure why that was needed before.

@elliette
Copy link
Member

Note that we will need to update the DevTools copybara script as well, otherwise this will be break DevTools in google3. This is where we replace bootstrapAppFor3P with bootstrapAppFor1P: cl/513846717

@kenzieschmoll kenzieschmoll merged commit 694d552 into flutter:master Apr 29, 2024
elliette added a commit to elliette/devtools that referenced this pull request May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate to new flutter bootstrapping

3 participants