-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Media: Add hooks and extension points for client-side media processing #74913
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
Changes from all commits
ee7a5fc
2f0323a
caec35c
76b6327
4be9e06
790b7b2
a9fcb88
1ab2a13
6c906f0
0e4e94b
88c9e42
ef9587b
adf0fb8
3cf7727
07edb0f
499dd0c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| https://github.com/WordPress/wordpress-develop/pull/11168 | ||
|
|
||
| * https://github.com/WordPress/gutenberg/pull/74913 |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| /** | ||
| * WordPress dependencies | ||
| */ | ||
| import apiFetch from '@wordpress/api-fetch'; | ||
|
|
||
| export default async function mediaFinalize( id ) { | ||
| await apiFetch( { | ||
| path: `/wp/v2/media/${ id }/finalize`, | ||
| method: 'POST', | ||
| } ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| /** | ||
| * WordPress dependencies | ||
| */ | ||
| import apiFetch from '@wordpress/api-fetch'; | ||
|
|
||
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import mediaFinalize from '..'; | ||
|
|
||
| jest.mock( '@wordpress/api-fetch', () => jest.fn() ); | ||
|
|
||
| describe( 'mediaFinalize', () => { | ||
| beforeEach( () => { | ||
| jest.clearAllMocks(); | ||
| } ); | ||
|
|
||
| it( 'should call the finalize endpoint with the correct path and method', async () => { | ||
| apiFetch.mockResolvedValue( {} ); | ||
|
|
||
| await mediaFinalize( 123 ); | ||
|
|
||
| expect( apiFetch ).toHaveBeenCalledWith( { | ||
| path: '/wp/v2/media/123/finalize', | ||
| method: 'POST', | ||
| } ); | ||
| } ); | ||
|
|
||
| it( 'should propagate errors from apiFetch', async () => { | ||
| apiFetch.mockRejectedValue( new Error( 'Network error' ) ); | ||
|
|
||
| await expect( mediaFinalize( 456 ) ).rejects.toThrow( 'Network error' ); | ||
| } ); | ||
| } ); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,6 @@ import { v4 as uuidv4 } from 'uuid'; | |
| */ | ||
| import { createBlobURL, isBlobURL, revokeBlobURL } from '@wordpress/blob'; | ||
| import type { createRegistry } from '@wordpress/data'; | ||
|
|
||
| type WPDataRegistry = ReturnType< typeof createRegistry >; | ||
|
|
||
| /** | ||
|
|
@@ -74,6 +73,7 @@ type ActionCreators = { | |
| rotateItem: typeof rotateItem; | ||
| transcodeImageItem: typeof transcodeImageItem; | ||
| generateThumbnails: typeof generateThumbnails; | ||
| finalizeItem: typeof finalizeItem; | ||
| updateItemProgress: typeof updateItemProgress; | ||
| revokeBlobUrls: typeof revokeBlobUrls; | ||
| < T = Record< string, unknown > >( args: T ): void; | ||
|
|
@@ -380,6 +380,15 @@ export function processItem( id: QueueItemId ) { | |
| return; | ||
| } | ||
|
|
||
| // If parent has pending operations (like Finalize), trigger them. | ||
| if ( | ||
| parentItem.operations && | ||
| parentItem.operations.length > 0 | ||
| ) { | ||
| dispatch.processItem( parentId ); | ||
| return; | ||
| } | ||
|
|
||
| if ( attachment ) { | ||
| parentItem.onSuccess?.( [ attachment ] ); | ||
| } | ||
|
|
@@ -403,6 +412,14 @@ export function processItem( id: QueueItemId ) { | |
| return; | ||
| } | ||
|
|
||
| // For Finalize, wait until all child sideloads are complete. | ||
| if ( | ||
| operation === OperationType.Finalize && | ||
| select.hasPendingItemsByParentId( id ) | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| dispatch< OperationStartAction >( { | ||
| type: Type.OperationStart, | ||
| id, | ||
|
|
@@ -446,6 +463,10 @@ export function processItem( id: QueueItemId ) { | |
| case OperationType.ThumbnailGeneration: | ||
| dispatch.generateThumbnails( id ); | ||
| break; | ||
|
|
||
| case OperationType.Finalize: | ||
| dispatch.finalizeItem( id ); | ||
| break; | ||
adamsilverstein marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| }; | ||
| } | ||
|
|
@@ -735,7 +756,8 @@ export function prepareItem( id: QueueItemId ) { | |
|
|
||
| operations.push( | ||
| OperationType.Upload, | ||
| OperationType.ThumbnailGeneration | ||
| OperationType.ThumbnailGeneration, | ||
| OperationType.Finalize | ||
| ); | ||
| } else { | ||
| operations.push( OperationType.Upload ); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When not dealing with an image I assume we don't need to also fire the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right! Finalize is only needed for images since it triggers |
||
|
|
@@ -1108,6 +1130,11 @@ export function generateThumbnails( id: QueueItemId ) { | |
| attachment.missing_image_sizes && | ||
| attachment.missing_image_sizes.length > 0 | ||
| ) { | ||
| const settings = select.getSettings(); | ||
| const allImageSizes = settings.allImageSizes || {}; | ||
| const sizesToGenerate: string[] = | ||
| attachment.missing_image_sizes as string[]; | ||
|
|
||
| // Use sourceFile for thumbnail generation to preserve quality. | ||
| // WordPress core generates thumbnails from the original (unscaled) image. | ||
| // Vips will auto-rotate based on EXIF orientation during thumbnail generation. | ||
|
|
@@ -1116,8 +1143,6 @@ export function generateThumbnails( id: QueueItemId ) { | |
| : item.sourceFile; | ||
| const batchId = uuidv4(); | ||
|
|
||
| const settings = select.getSettings(); | ||
| const allImageSizes = settings.allImageSizes || {}; | ||
| const { imageOutputFormats } = settings; | ||
|
|
||
| // Check if thumbnails should be transcoded to a different format. | ||
|
|
@@ -1141,7 +1166,7 @@ export function generateThumbnails( id: QueueItemId ) { | |
| ); | ||
| } | ||
|
|
||
| for ( const name of attachment.missing_image_sizes ) { | ||
| for ( const name of sizesToGenerate ) { | ||
| const imageSize = allImageSizes[ name ]; | ||
| if ( ! imageSize ) { | ||
| // eslint-disable-next-line no-console | ||
|
|
@@ -1255,6 +1280,40 @@ export function generateThumbnails( id: QueueItemId ) { | |
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Finalizes an uploaded item by calling the server's finalize endpoint. | ||
| * | ||
| * This triggers the wp_generate_attachment_metadata filter so that PHP | ||
| * plugins can process the attachment after all client-side operations | ||
| * (including thumbnail sideloads) are complete. | ||
| * | ||
| * @param id Item ID. | ||
| */ | ||
| export function finalizeItem( id: QueueItemId ) { | ||
| return async ( { select, dispatch }: ThunkArgs ) => { | ||
| const item = select.getItem( id ); | ||
| if ( ! item ) { | ||
| return; | ||
| } | ||
|
|
||
| const attachment = item.attachment; | ||
| const { mediaFinalize } = select.getSettings(); | ||
|
|
||
| // Only finalize if we have an attachment ID and a mediaFinalize callback. | ||
| if ( attachment?.id && mediaFinalize ) { | ||
| try { | ||
| await mediaFinalize( attachment.id ); | ||
| } catch ( error ) { | ||
| // Log but don't fail the upload if finalization fails. | ||
| // eslint-disable-next-line no-console | ||
| console.warn( 'Media finalization failed:', error ); | ||
| } | ||
| } | ||
|
|
||
| dispatch.finishOperation( id, {} ); | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Revokes all blob URLs for a given item, freeing up memory. | ||
| * | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a question, but I was wondering if we want
finalizeto be only for the filter, or if we'd want to callwp_update_image_subsizesas a catch all?I'm very possibly overthinking this, but if for some reason the client-side processing didn't generate everything it needed to, I was wondering if the
wp_update_image_subsizesfunction could cover things off on the server-side.That said, I'm suspicious of my own comment here, so mostly just wanted to capture the thought in case there's anything there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The intention here is that
wp_generate_attachment_metadatagets triggered only when all of the image processing and uploading is completed successfully. We shouldn't need to generate additional sizes once this is called.If client side generation fails to generate all of the sub sizes for any reason, we will continue that process at the next possible opportunity (when the editor is open & connected on the same post).