Skip to content

Send RPC request to switch assets directory on hot reload. - #12872

Merged
aam merged 9 commits into
flutter:masterfrom
aam:switch-assets-on-hot-reload
Jan 4, 2018
Merged

aam merged 9 commits into
flutter:masterfrom
aam:switch-assets-on-hot-reload

Conversation

@aam

@aam aam commented Nov 3, 2017

Copy link
Copy Markdown
Member

This is needed to pick up updated assets that are expected to be picked up on hot reload.

This relies on flutter-team-archive/engine#4323

This is needed to pick up updated assets that are expected to be picked up on hot reload.
@aam
aam requested a review from a-siva November 3, 2017 21:42
}

Future<Null> setAssetDirectory(Uri assetsDirectory) async {
if (assetsDirectory != null) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

when will the assetsDirectory be null?
I am wondering if it is sufficient to assert that assetsDirectory is not null.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good point, done.


Future<Null> resetAssetDirectory() async {
final Uri deviceAssetsDirectoryUri = devFS.baseUri.resolveUri(
fs.path.toUri(getAssetBuildDirectory()));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

assert deviceAssetsDirectoryUri != null ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done

@Hixie

Hixie commented Nov 22, 2017

Copy link
Copy Markdown
Contributor

cc @cbracken for review

assert(deviceAssetsDirectoryUri != null);
for (FlutterView view in views) {
await view.setAssetDirectory(deviceAssetsDirectoryUri);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe very slightly better?

await Future.wait(views.map((view) => view.setAssetDirectory(deviceAssetsDirectoryUri)));

final Uri deviceAssetsDirectoryUri = devFS.baseUri.resolveUri(
fs.path.toUri(getAssetBuildDirectory()));
assert(deviceAssetsDirectoryUri != null);
await Future.wait(views.map((view) => view.setAssetDirectory(deviceAssetsDirectoryUri)));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

FlutterView view

@cbracken

Copy link
Copy Markdown
Member

lgtm

@aam

aam commented Nov 30, 2017

Copy link
Copy Markdown
Member Author

Currently this is pending on flutter-team-archive/engine#4323 being reviewed and landed.

@aam
aam merged commit 8da5af5 into flutter:master Jan 4, 2018
@aam
aam deleted the switch-assets-on-hot-reload branch January 4, 2018 22:27
@Hixie

Hixie commented Jan 5, 2018

Copy link
Copy Markdown
Contributor

This seems to have dramatically regressed some of the hot reload benchmarks.

@aam

aam commented Jan 5, 2018

Copy link
Copy Markdown
Member Author

There is something fishy going on with those benchmarks. See discussion on original commit: 8da5af5#commitcomment-26676085

DaveShuckerow pushed a commit to DaveShuckerow/flutter that referenced this pull request May 14, 2018
…2872)

* Send RPC request to switch assets directory on hot reload.

This is needed to pick up updated assets that are expected to be picked up on hot reload.

* Assert assets directory is not null.

* Better multiple future wait

* Add type annotation
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants