Blazor Image component to display images that are not accessible through HTTP endpoints

javiercn commented 4 years ago

Summary

Introduce an Image component to display images coming from dynamic sources like API calls or databases.

Motivation

Currently the most common way to display an image coming from the Database is to base64 encoded it and create an object URL for it. This is problematic since images have a big size and that results in the allocation of large strings, which has a big impact on performance. We want to display images on the browser without allocating large amounts of memory on the .NET side.

Goals

Simplify rendering images that come from sources that are not addressable via URLs.
Prevent users from allocating large amounts of memory on their applications to display images.

Non-goals

Implement responsive images or advanced functionality available in HTML like srcset and Picture

Scenarios

As a developer I want to display a profile picture for my users that comes from the database.
```
<Image Source="_imageSource" />
```

@code { private ImageSource _imageSource;

protected override OnInitializedAsync()
{
    var data = await client.GetProfileAsync();
    _imageSource = new ImageSource("img/jpg", "profile.jpg", data);
}

}



## Risks
* Customers might not discover the feature:
  * We can limit that with docs, and maybe an "analyzer like" warning.

## Interaction with other parts of the framework

## Detailed design
This is a rough sketch, but the idea is to create an object on the JS side attached to the image. Use JS interop or other means to transfer the image down to the client (at which points its not longer on the .NET memory) and display it.

## Drawbacks
We'll likely want to cache these things in the browser and will have to handle what happens when the input data changes and so on in the server.
We'll likely also want to avoid performing unnecessary requests on the server to retrieve an image  that has already been retrieved, which might complicate things.

## Considered alternatives
The current alternatives are:
* Base64 encode the image and add it to the source element.
  * This allocates large strings which we are trying to avoid.
* Create a separate endpoint on the server to retrieve and serve the image specifically.
  * This can interfere with the authorization rules in your app since the browser is making the request and might not have access to the credentials you need to call the API.
  * In general it requires much more work and knowledge to get it right.

## Open questions
TBD

ghost commented 4 years ago

Thanks for contacting us. We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

ghost commented 4 years ago

Thanks for contacting us. We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

mihaimyh commented 4 years ago

This sounds like a really nice feature.

SteveSandersonMS commented 4 years ago

We could consider this a special case of "support passing byte[] in JS interop calls without base64 serialization". If we supported that, then it would be trivial to convert a byte[] to a blob URL, which could then be used on an <img src>.

ghost commented 4 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

dennisbdannunzio-leonardo commented 4 years ago

This is a useful feature. But, the "support passing byte[]" in JS interop calls is a better generalization and more flexible approach.

javiercn commented 3 years ago

https://github.com/dotnet/aspnetcore/issues/27802

steamonimo commented 3 years ago

This would be benefitial to reduce the attack surface of web applications: no additional rest interface needed for image creation. We currently use base64 encoded images and it would be nice to reduce the memory overhead for this.

javiercn commented 3 years ago

https://github.com/dotnet/aspnetcore/issues/28110

javiercn commented 3 years ago

Another instance of this https://github.com/dotnet/aspnetcore/issues/31585

TanayParikh commented 2 years ago

In the interim a new docs page covering best practices for working with images in Blazor (in .NET 6) is available here: https://docs.microsoft.com/aspnet/core/blazor/images?view=aspnetcore-6.0

TanayParikh commented 2 years ago

Thanks all for the discussion.

We could consider this a special case of "support passing byte[] in JS interop calls without base64 serialization". If we supported that, then it would be trivial to convert a byte[] to a blob URL, which could then be used on an <img src>.

This is exactly the scenario https://docs.microsoft.com/aspnet/core/blazor/images?view=aspnetcore-6.0 covers. Closing out this issue accordingly.

mkArtakMSFT commented 2 years ago

Reopening as after some discussion within the team we believe there will still be value from having a separate components for this.

smartprogrammer93 commented 2 years ago

this have huge value for enterprise applications for this. we use base64 heavily in our application now. i see this as a huge improvement to performance.

CommonLoon102 commented 2 years ago

This would definitely make things much easier. Go for the component.

javiercn commented 2 years ago

We are going to be using this issue to track https://github.com/dotnet/aspnetcore/issues/30290. In 6.0 we added support for dealing with streams of data in 6.0

As part of this issue, we want to add some component primitives that simplify some common scenarios.

We want to have some sort of MediaComponent base component that:
- Receives a stream as a parameter.
- Converts the stream into an object URL.
- Handles the lifetime of the object URL (meaning it revokes it under certain circumstances).
- Might potentially support caching the content on the client by providing a key, to avoid repeated roundtrips if the content was already sent once.
An Image component that uses the object URL in the component to display an image.
A Video component that uses the object URL in the component to display a video.
Optionally, a FileDownload component counterpart to FileUpload that allows the user to save the file.

All this builds on top of the APIs we already provided in 6.0 to deal with large binary data.

TanayParikh commented 2 years ago

Might potentially support caching the content on the client by providing a key, to avoid repeated roundtrips if the content was already sent once.

SteveSandersonMS commented 2 years ago

This definitely sounds like a good feature to me!

I've been looking for some way to have an objectURL representing a ReadableStream or Response that is still in flight, but can't see any evidence it's possible (other than using a service worker, which I don't think we want to introduce here). So it looks like we'd have to make the client read the entire stream into memory as a blob, and then construct an objectURL from that. This is better than the "base64" alternative but is still a bit limited as:

There isn't a way to see the image arriving progressively, unlike a normal <img src=...>
It's definitely not suitable for any kind of video

However this does simplify the feature a bit, since all we can do is use the existing JS binary streaming interop to send a whole byte[]/Stream to JS and then get it to construct a blob and then objectURL from that. We'd probably then use a MutationObserver to revoke the objectURL as soon as the img is removed or if its src changes.

Might potentially support caching the content on the client by providing a key, to avoid repeated roundtrips if the content was already sent once.

We could theoretically construct a fake Response object and then store it via the Cache Storage API. This would involve more orchestration, as the JS side would have to pull the data if it's not already cached, instead of more simply having the .NET side push it regardless. It could certainly be done. We'd want to have some design around the cache rules (e.g., expiry) and to what extent we're interested in the .NET side being able to push updates that override the client-side cache.

Optionally, a FileDownload component counterpart to FileUpload that allows the user to save the file.

That sounds good. I suspect it wouldn't be a component at all, but rather just a helper method like:

await FileDownload.SaveAs(streamOrByteArray, "filename.ext");

... since you'd trigger it procedurally when something happens, such as the user clicking a button.

SteveSandersonMS commented 2 years ago

We also need to think about what the img elements should display before the image data has been fully transferred. Presumably we don't want it to display as a broken image. So do we just not add the img element to the document until we have the data? Or do we render a <div> that has the expected width/height to avoid weird reflow effects as the images load?

Update: I suggest we do render an <img> element right away, but set its src to some kind of "blank image" value (maybe as a data URL). That way, if you have a CSS rule that matches img, it will apply both before and after the image data is fetched.

javiercn commented 2 years ago

It's definitely not suitable for any kind of video

I wouldn't focus too much on Video vs Image and more on the size as a trait, you can also have images that take in a GB and we can always make a comment that this is designed for images/videos up to x MB, which is something that we can measure and make that clear in the remarks. There are plenty of short clips that this is useful for (an example would be mp4 gifs).

We could theoretically construct a fake Response object and then store it via the Cache Storage API. This would involve more orchestration, as the JS side would have to pull the data if it's not already cached, instead of more simply having the .NET side push it regardless. It could certainly be done. We'd want to have some design around the cache rules (e.g., expiry) and to what extent we're interested in the .NET side being able to push updates that override the client-side cache.

I was thinking of a simpler approach where you can pass a URL and do the caching/invalidation yourself by modifying the fake URL. I don't think we need to be doing cleanup as I expect the browser to automatically cleanup when the space limit is reached (if necessary). We would need to do some investigation, but I wouldn't worry about cleanup too much.

The main goal here is to avoid recurring transfers on subsequent visits/re-renders which can also make it much faster to render.

I've been looking for some way to have an objectURL representing a ReadableStream or Response that is still in flight, but can't see any evidence it's possible (other than using a service worker, which I don't think we want to introduce here). So it looks like we'd have to make the client read the entire stream into memory as a blob, and then construct an objectURL from that. This is better than the "base64" alternative but is still a bit limited as:

Unfortunately, this is not possible today :(.

SteveSandersonMS commented 2 years ago

I wouldn't focus too much on Video vs Image and more on the size as a trait

I'd expect that:

If we make a component that displays images, web developers will expect it to work with the kinds of images that typically are used on the web
If we make a component that displays videos, web developers will expect it to work with the kinds of videos that typically are used on the web

With the design as given, (1) would work but (2) would not. It would really only be a specialised kind of video that would work, and in general we'd be guiding people towards failure except in these special cases. The great majority of video scenarios on the web rely on streaming to make the behavior reasonable. Without evidence that customers are asking for a video feature and their scenarios are exclusively very tiny video files, this seems like a fairly clear-cut thing to exclude.

I was thinking of a simpler approach where you can pass a URL and do the caching/invalidation yourself by modifying the fake URL.

OK, cool! I was just trying to think of where the cache data actually gets stored. If it goes in the Cache Storage API, then we have to make some decision about the caching headers we put on the fake Response objects that get cached. If the decision is "don't put any caching headers and let the browser do whatever it wants" that's totally fine and reasonable, especially if we have some sense of what the practical behavior will be as the cache gets filled up. If you were thinking we use some other storage than Cache Storage, I'd be interested in what.

javiercn commented 2 years ago

this seems like a fairly clear-cut thing to exclude.

I'm fine with that, I don't feel super strongly about it. Will you be ok if we do things in a way that would allow people to create their own component leveraging our infrastructure? (something like a base class or similar).

FYI, for videos you can actually create a MediaStream object and assign it to the srcObject property, which doesn't involve putting the entire thing in memory I believe. It is only images that don't have this ability.

SteveSandersonMS commented 2 years ago

FYI, for videos you can actually create a MediaStream object and assign it to the srcObject property,

Interesting, I didn't know that. Sounds like that would involve supplying MediaStream content programmatically or do this, which I didn't see any API for (but only looked for a few minutes, so might have missed something obvious).

I'm fine with that, I don't feel super strongly about it. Will you be ok if we do things in a way that would allow people to create their own component leveraging our infrastructure? (something like a base class or similar).

I have no objection to letting people subclass the component we provide and overriding the BuildRenderTree method so that it produces a <video> element instead of an <img> element. They should be able to do this easily with a regular .razor file and @inherits. My concern is only about deliberately suggesting to people that they should use this for videos when we know in most cases it will be substandard vs normal streaming :)

javiercn commented 2 years ago

I have no objection to letting people subclass the component we provide and overriding the BuildRenderTree method so that it produces a <video> element instead of an <img> element. They should be able to do this easily with a regular .razor file and @inherits. My concern is only about deliberately suggesting to people that they should use this for videos when we know in most cases it will be substandard vs normal streaming :)

I agree, I just want to make sure that the guts of our component can be reused so that things like caching, etc. don't have to be reimplemented. That is why I was suggesting we had a "base" Media component and a concrete Image component that extends it for use with images.

The other thing that might be interesting to consider (somewhat orthogonal) is if there is a way in which we could make the JS this component needs, separate from the main Blazor bundle. Maybe something similar to what we tell customers to do with JS modules. It'll be interesting to see if we can establish a modular pattern for these features so that they are only paid for when used.

SteveSandersonMS commented 2 years ago

The other thing that might be interesting to consider (somewhat orthogonal) is if there is a way in which we could make the JS this component needs, separate from the main Blazor bundle

Very interesting. Earlier, I was contemplating suggesting making a separate package for this (for the same reason) but didn't because I didn't want to overcomplicate things. If we could get the JS into a separate bundle and rely on the .NET trimmer for taking out unused .NET code, then we'd get the size reduction without needing a separate package which would be better still.

I guess the work here would be figuring out how to extend our build system in a clear enough and reusable enough way. Maybe it's fairly straightforwards, in that we already build multiple blazor.*.js files (using different webpack entrypoints) so maybe we could have another webpack entrypoint for blazor.mediacomponent.js or whatever it gets called. We'd also have to make sure it gets into the right packages so it can be served from _framework or similar.

javiercn commented 2 years ago

@SteveSandersonMS I would imagine that we would define these as a JS module and produce a separate output, and then we would need to make sure the SDK accounts for this file in the same way as the blazor.webassembly.js file.

Now that I think about it is a bit more complicated, since we would need to figure things out for Blazor Server (I guess we would also embed the file in that case) and blazor webview. I am inclined to say, keep it simple 😄. We can figure out how to do this in the future, as I think it is interesting to have a chance to "ship these things inbox" but following a similar pattern as a third-party library.

ghost commented 2 years ago

Thanks for contacting us.

We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

danroth27 commented 2 years ago

While we have made progress on this feature, at this point we think the risk is too high to include this in the .NET 7 release. Moving to .NET 8.

javiercn commented 1 year ago

Another instance this component is useful https://github.com/dotnet/aspnetcore/issues/47449

javiercn commented 1 year ago

Another problematic case https://github.com/dotnet/aspnetcore/issues/48123

ghost commented 1 year ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

dotnet / aspnetcore