-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Add API for discovering assets #116621
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add API for discovering assets #116621
Conversation
|
@jonahwilliams This is still WIP but it can be reviewed from a design/approach standpoint. |
|
Flutter generally uses a layered architecture. What this means more specifically is that when we have one system (say manifest loading and parsing) that needs to talk to another system (say an image widget/loader), we try to design this system so that these systems communicate via a public API. There are a number of advantages to this approach:
So given that, I would design the |
|
aghhh "I would " -> "I wouldn't" |
|
Thanks for the detailed response!
I agree that the exposed manifest data being weakly typed is far from ideal, and providing a stronger concrete type for manifest entry values has notable benefits1. I've implemented this in my latest commit with the following changes:
One tradeoff is that the shape of the asset manifest data would become harder to change.
Interesting point. Is the idea that custom Footnotes
|
|
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
(Triage): @andrewkolos Do you still have plans for this PR? |
|
@goderbauer Sorry for letting this go stale. This PR will be superseded by #118410, which I will mark as ready-for-review very soon. |
Overview
This change introduces a public API for inspecting what assets are available within a Flutter app. Currently, there is no officially-supported way to do this, and some apps--needing this ability--depend on undocumented behavior internal to Flutter.
Example use case
The google_fonts package, when loading a font, first checks if the font is already available in the app. If it is not available, it will load it from fonts.google.com. Code.
Other known use cases:
Context
Terminology
The asset manifest. The framework, when performing it's first asset load, has to load a file called the asset manifest, which contains information about assets available to the app (e.g. where to find higher-resolution variants of images). This file is created at build time by the tool. It is meant for framework-internal use--but some Flutter apps, including ones internal to google, read the file directly.
Why we should make this change now
#113637 "replaced" the aforementioned asset manifest (hereafter "legacy manifest") with a new one that enables more performant first loads and has a more flexible schema. However, this PR did not remove all references to the legacy manifest file, since doing so would break apps that directly depend on it1. In addition, g3 Flutter does not yet generate the new asset manifest file, so--when loading an asset--the framework needs to be able to fallback to the legacy manifest file in the event that the new one isn't present (see #114913).
Still, we do ultimately want to get rid of the legacy asset manifest (#114913). Rather than having users migrate to reading a new framework-internal file, we can use this opportunity to introduce a dedicated, publicly-documented API for listing what assets are available. Users can instead migrate to this new API. This results in less hacky user code and also gives Flutter maintainers more freedom/flexibility to change the internals of asset loading without worrying about breaking user code.
Design
We add a new class,
AssetManifest, that is responsible for loading and parsing the asset manifest file contents from anAssetBundleby callingloadStructuredBinaryData(andloadStructuredDataas a fallback in case only the legacy asset manifest file is available). This loading will invoked via static factory methods (e.g.AssetManifest.loadFromAssetBundle).AssetManifestwill expose two instance methods,listAssetsandgetAssetMetadata.Iterable<String> listAssets()will simply enumerate the list of assets in the manifest by their keys. This provides the capability that we know users need.Object? getAssetMetadata(String key)will get the value of the manifest entry corresponding to the asset having the given key. This is meant for use by the framework (i.e. for choosing which image asset variants to load). Since this API is public, the return type is intentionally vague to discourage user code from depending on its shape2.Drawbacks
We really want the framework be the only one to ever calling
getAssetMetadata. While the return type being vague discourages users from depending on the shape of returned data, it doesn't stop them from inspecting it at runtime and depending on hardcoded object keys. However, there's no clear alternative to having this method. Here's one I can think of: havelistAssets(which can be moved toAssetBundleif desired) and the existing image resolution logic both independently load the asset manifest file. The parser they use will be have to be identical, since the first parsed result could be cached withinAssetBundle.In addition, both will need to support logic for falling back to the legacy asset manifest, which would be a lot of duplicated (if temporary) code.Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Footnotes
Normally, it might be fair to perform such a breaking change, because the asset manifest is not a publicly documented/supported concept. However, it's possible that some users, when consulting with Flutter maintainers, were told about the file and that they could use it to accomplish the asset-related behavior they were looking for. As a result, it would be unfair to break the older behavior without a full deprecation cycle. ↩
It is difficult to use a type other than
Object?for the map entry value type unless we commit to the idea that values only ever represent list of image variants. ↩