Conversation
… use sync resolution
There was a problem hiding this comment.
Code Review
This pull request introduces asynchronous image source resolution to the any_image package, enabling MIME sniffing for extension-less URLs via the new MimeResolver and AsyncSourceResolver. The ResolverPipeline has been updated to handle both sync and async paths, and the AnyImage widget was converted to a StatefulWidget to manage this process. Feedback focuses on improving the robustness of the MimeResolver with timeouts and status checks, ensuring AnyImage correctly re-resolves when its configuration changes, and optimizing the FutureBuilder with initialData to prevent flickering for synchronous sources.
| void didUpdateWidget(AnyImage oldWidget) { | ||
| super.didUpdateWidget(oldWidget); | ||
| if (oldWidget.source != widget.source) { | ||
| _resolved = _resolve(); | ||
| } | ||
| } |
There was a problem hiding this comment.
The didUpdateWidget implementation only triggers a re-resolution when the source changes. However, the widget's behavior also depends on the format override and the pipeline configuration. If either of these properties is updated by the parent widget while the source remains the same, the image will not reflect the changes. You should include these fields in the update check.
@override
void didUpdateWidget(AnyImage oldWidget) {
super.didUpdateWidget(oldWidget);
if (oldWidget.source != widget.source ||
oldWidget.format != widget.format ||
oldWidget.pipeline != widget.pipeline) {
_resolved = _resolve();
}
}| final response = await http.head(Uri.parse(source)); | ||
| final contentType = response.headers['content-type']; |
There was a problem hiding this comment.
The network request lacks a timeout and does not verify the HTTP status code. This can lead to the widget hanging in a loading state indefinitely if the server is unresponsive, or incorrectly resolving a source if the server returns an error page (e.g., 404) with a valid image Content-Type header. It is recommended to add a timeout and check for a 200 OK response.
| final response = await http.head(Uri.parse(source)); | |
| final contentType = response.headers['content-type']; | |
| final response = await http.head(Uri.parse(source)).timeout( | |
| const Duration(seconds: 10), | |
| ); | |
| if (response.statusCode != 200) return null; | |
| final contentType = response.headers['content-type']; |
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| return FutureBuilder<ResolvedSource>( |
There was a problem hiding this comment.
Using FutureBuilder without initialData introduces a mandatory one-frame delay (asynchrony) before the image is rendered, even for synchronous sources like local assets or URLs with extensions. This results in a visible flicker where the placeholder is shown briefly whenever the source changes.
To maintain the performance of the previous synchronous implementation, consider providing initialData by calling widget.pipeline.resolveSync(widget.source) (and applying the widget.format override). This ensures that if the source can be resolved immediately, it renders without delay.
Summary
Adds MIME-type sniffing as a fallback classification strategy for sources that cannot be resolved by prefix or extension alone (e.g. CDN URLs, signed URLs, or proxy endpoints with no file extension).
Closes #1
Type of Change
Checklist
dart format --set-exit-if-changed .passesflutter analyzepasses with no warningsflutter testpassesCHANGELOG.md