fix(storage-resizes-images) Fixes issue where resized image is not rendered in Firebase Console (Issue #140) #279
Conversation
|
@firebase/invertase Can you verify that this works? If the option is set to "false" then does it allow the resized image to be downloaded? If yes, then it seems that this should not be a param, but the download token should just always be preserved, and we would make a minor version bump. |
|
No, well I mean right now you cant download the resized images, so defaulting to false I assumed would not break existing configurations for users who have it working this way now and for some reason want it like that? I agree 100%, it shouldn't be a param, and the delete command should just go away honestly... but I didn't want to rock the boat. Like I can't for the life of my understand why this is here Wouldn't it be the inverse, more people would want that access token there so the resized image just works? |
That's why I suggested a minor version bump, which for versions under 1.0.0 indicates a breaking change. (I think this is worth doing a breaking change for, rather than adding an unnecessary param) |
|
Ok, so if I have the blessing... remove the param and delete entirely, bump to 0.2.0? Sound good? |
|
@sitefinitysteve Yes please, also you do not have to increment the version or add to the changelog in this PR, that gets automatically generated later when we do the release. Thanks for your contribution! |
laurenzlong
changed the title
laurenzlong
changed the title
| @@ -161,7 +164,6 @@ const resizeImage = ({ bucket, originalFile, fileDir, fileNameWithoutExtension, | |||
| else { | |||
| metadata.cacheControl = objectMetadata.cacheControl; | |||
| } | |||
| delete metadata.metadata.firebaseStorageDownloadTokens; | |||
| // Generate a resized image using Sharp. | |||
There was a problem hiding this comment.
Ah of course, line 164 the source of all our pain. :-)
|
Hey @laurenzlong, I can confirm that this works. This is a resized image before the change is made. As you can see there is no image displayed because it need a new token creating: This is a resized image after the change. Image is displayed because the token is preserved: |
|
I mean i've been using this fix by editing the cloud function directly for a few weeks, but new releases keep blowing away the edit, I know she works. On that note, what's the release schedule like for something like this? |
|
Thanks @sitefinitysteve and @russellwheatley! This is failing the linter, could you please fix by running We can slot this in for the May 7 release. |
|
Done, thanks @laurenzlong and team, this saved me big time! |
|
Thanks for the contribution! |
The problem with the current extension is outlined here
This should add a new configuration option to NOT delete the token, defaults to true to preserve existing functionality, users can go in to disable it if they don't want it happening.