Skip to content

Commit fa1b048

Browse files
andrewserongramonjdMamaduka
authored
Image Block: Remove chained entity record calls (#79469)
Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
1 parent 122a29e commit fa1b048

2 files changed

Lines changed: 44 additions & 68 deletions

File tree

‎packages/block-library/src/image/test/use-open-image-media-editor-modal.js‎

Lines changed: 20 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -45,16 +45,14 @@ jest.mock( '../../lock-unlock', () => ( {
4545

4646
function createRegistry( {
4747
getEditedEntityRecord = () => false,
48-
getEntityRecord = () => undefined,
49-
resolveGetEntityRecord = getEntityRecord,
48+
resolveGetEntityRecord = () => undefined,
5049
} = {} ) {
5150
const actions = {
5251
invalidateResolution: jest.fn(),
5352
};
5453
return {
5554
select: jest.fn( () => ( {
5655
getEditedEntityRecord,
57-
getEntityRecord,
5856
} ) ),
5957
dispatch: jest.fn( () => actions ),
6058
resolveSelect: jest.fn( () => ( {
@@ -169,25 +167,24 @@ describe( 'useOpenImageMediaEditorModal', () => {
169167
caption: 'Original caption',
170168
},
171169
registryOptions: {
172-
getEntityRecord: () => originalAttachment,
173-
resolveGetEntityRecord: ( kind, name, attachmentId, query ) =>
174-
query?.context === 'edit'
175-
? updatedAttachment
176-
: originalAttachment,
170+
getEditedEntityRecord: () => originalAttachment,
171+
resolveGetEntityRecord: () => updatedAttachment,
177172
},
178173
} );
179174

180175
expect( setAttributes ).toHaveBeenCalledWith( {
181176
alt: 'Updated alt',
182177
caption: 'Updated caption',
183178
} );
184-
expect( registry.actions.invalidateResolution ).toHaveBeenCalledWith(
185-
'getEntityRecord',
186-
[ 'postType', 'attachment', 1 ]
179+
// A single, query-less resolution is invalidated: the hook reads,
180+
// resolves, and invalidates the attachment through the entity's default
181+
// (edit) context rather than an explicitly-keyed query.
182+
expect( registry.actions.invalidateResolution ).toHaveBeenCalledTimes(
183+
1
187184
);
188185
expect( registry.actions.invalidateResolution ).toHaveBeenCalledWith(
189186
'getEntityRecord',
190-
[ 'postType', 'attachment', 1, { context: 'edit' } ]
187+
[ 'postType', 'attachment', 1 ]
191188
);
192189
} );
193190

@@ -220,8 +217,7 @@ describe( 'useOpenImageMediaEditorModal', () => {
220217
1,
221218
'postType',
222219
'attachment',
223-
1,
224-
{ context: 'edit' }
220+
1
225221
);
226222
expect( openMediaEditorModal ).toHaveBeenCalledWith( {
227223
id: 1,
@@ -263,8 +259,7 @@ describe( 'useOpenImageMediaEditorModal', () => {
263259
1,
264260
'postType',
265261
'attachment',
266-
1,
267-
{ context: 'edit' }
262+
1
268263
);
269264
expect( openMediaEditorModal ).toHaveBeenCalledWith( {
270265
id: 1,
@@ -299,7 +294,7 @@ describe( 'useOpenImageMediaEditorModal', () => {
299294
caption: undefined,
300295
},
301296
registryOptions: {
302-
getEntityRecord: () => ( {
297+
getEditedEntityRecord: () => ( {
303298
id: 1,
304299
alt_text: '',
305300
caption: {
@@ -314,8 +309,7 @@ describe( 'useOpenImageMediaEditorModal', () => {
314309
1,
315310
'postType',
316311
'attachment',
317-
1,
318-
{ context: 'edit' }
312+
1
319313
);
320314
expect( setAttributes ).toHaveBeenCalledWith( {
321315
caption: 'Updated attachment caption',
@@ -341,7 +335,7 @@ describe( 'useOpenImageMediaEditorModal', () => {
341335
caption: '',
342336
},
343337
registryOptions: {
344-
getEntityRecord: ( kind, name, attachmentId ) =>
338+
getEditedEntityRecord: ( kind, name, attachmentId ) =>
345339
attachmentId === 1 ? originalAttachment : undefined,
346340
resolveGetEntityRecord: ( kind, name, attachmentId ) =>
347341
attachmentId === 2 ? updatedAttachment : undefined,
@@ -371,7 +365,7 @@ describe( 'useOpenImageMediaEditorModal', () => {
371365
};
372366
const deferredAttachment = createDeferred();
373367
const registry = createRegistry( {
374-
getEntityRecord: ( kind, name, attachmentId ) =>
368+
getEditedEntityRecord: ( kind, name, attachmentId ) =>
375369
attachmentId === 1 ? originalAttachment : undefined,
376370
resolveGetEntityRecord: ( kind, name, attachmentId ) =>
377371
attachmentId === 2 ? deferredAttachment.promise : undefined,
@@ -437,7 +431,7 @@ describe( 'useOpenImageMediaEditorModal', () => {
437431
caption: '',
438432
},
439433
registryOptions: {
440-
getEntityRecord: ( kind, name, attachmentId ) =>
434+
getEditedEntityRecord: ( kind, name, attachmentId ) =>
441435
attachmentId === 1
442436
? originalAttachment
443437
: {
@@ -469,7 +463,7 @@ describe( 'useOpenImageMediaEditorModal', () => {
469463
caption: undefined,
470464
},
471465
registryOptions: {
472-
getEntityRecord: () => ( {
466+
getEditedEntityRecord: () => ( {
473467
id: 1,
474468
alt_text: '',
475469
caption: { raw: 'Existing caption' },
@@ -526,7 +520,7 @@ describe( 'useOpenImageMediaEditorModal', () => {
526520
caption: undefined,
527521
},
528522
registryOptions: {
529-
getEntityRecord: () => ( {
523+
getEditedEntityRecord: () => ( {
530524
id: 1,
531525
alt_text: 'Original alt',
532526
caption: { raw: 'Existing caption' },
@@ -556,7 +550,7 @@ describe( 'useOpenImageMediaEditorModal', () => {
556550
caption: { toString: () => '' },
557551
},
558552
registryOptions: {
559-
getEntityRecord: () => ( {
553+
getEditedEntityRecord: () => ( {
560554
id: 1,
561555
alt_text: 'Original alt',
562556
caption: { raw: 'Existing caption' },
@@ -603,7 +597,7 @@ describe( 'useOpenImageMediaEditorModal', () => {
603597
};
604598
const deferredAttachment = createDeferred();
605599
const registry = createRegistry( {
606-
getEntityRecord: () => ( {
600+
getEditedEntityRecord: () => ( {
607601
id: 1,
608602
alt_text: '',
609603
caption: { raw: '' },

‎packages/block-library/src/image/use-open-image-media-editor-modal.js‎

Lines changed: 24 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,6 @@ export function getSyncedImageBlockAttributes(
104104
}
105105

106106
const { openMediaEditorModalKey } = unlock( blockEditorPrivateApis );
107-
// Caption sync needs `caption.raw`; view/default attachment records can contain
108-
// only rendered caption data or be tied to an in-flight stale resolution.
109-
const ATTACHMENT_EDIT_QUERY = { context: 'edit' };
110107

111108
function getAttachmentFallbackForEmptyBlockMetadata( { alt, caption } ) {
112109
const attachment = {};
@@ -177,25 +174,23 @@ export function useOpenImageMediaEditorModal( {
177174
};
178175
}, [ alt, caption, id, url ] );
179176

177+
// Reads the cached attachment record. The `attachment` postType entity
178+
// fetches with `context: 'edit'` by default, so `getEditedEntityRecord`
179+
// returns the edit-context record — carrying `caption` as `{ raw }` and a
180+
// usable `alt_text` — without us specifying a context. It is resolver-
181+
// backed, so on a cold cache this also kicks off the fetch and returns a
182+
// falsy value synchronously; that resolution shares its cache key with the
183+
// `resolveAttachmentRecord` call below (both keyed on the no-query
184+
// `getEntityRecord`), so the two dedupe into a single request.
180185
const getCachedAttachmentRecord = useCallback(
181-
( attachmentId ) => {
182-
const { getEditedEntityRecord, getEntityRecord } =
183-
registry.select( coreStore );
184-
return (
185-
getEditedEntityRecord(
186+
( attachmentId ) =>
187+
registry
188+
.select( coreStore )
189+
.getEditedEntityRecord(
186190
'postType',
187191
'attachment',
188192
attachmentId
189-
) ||
190-
getEntityRecord(
191-
'postType',
192-
'attachment',
193-
attachmentId,
194-
ATTACHMENT_EDIT_QUERY
195-
) ||
196-
getEntityRecord( 'postType', 'attachment', attachmentId )
197-
);
198-
},
193+
),
199194
[ registry ]
200195
);
201196

@@ -204,18 +199,10 @@ export function useOpenImageMediaEditorModal( {
204199
const resolveSelect = registry.resolveSelect( coreStore );
205200

206201
try {
207-
return (
208-
( await resolveSelect.getEntityRecord(
209-
'postType',
210-
'attachment',
211-
attachmentId,
212-
ATTACHMENT_EDIT_QUERY
213-
) ) ||
214-
( await resolveSelect.getEntityRecord(
215-
'postType',
216-
'attachment',
217-
attachmentId
218-
) )
202+
return await resolveSelect.getEntityRecord(
203+
'postType',
204+
'attachment',
205+
attachmentId
219206
);
220207
} catch {
221208
return undefined;
@@ -226,21 +213,16 @@ export function useOpenImageMediaEditorModal( {
226213

227214
const resolveFreshAttachmentRecord = useCallback(
228215
async ( attachmentId ) => {
229-
// Bust cached records so resolveAttachmentRecord fetches the
230-
// server state that reflects the media editor's saved changes.
216+
// Invalidate the cached resolution so resolveAttachmentRecord
217+
// re-fetches the server state that reflects the media editor's
218+
// saved changes.
231219
const { invalidateResolution } = registry.dispatch( coreStore );
232220

233221
invalidateResolution( 'getEntityRecord', [
234222
'postType',
235223
'attachment',
236224
attachmentId,
237225
] );
238-
invalidateResolution( 'getEntityRecord', [
239-
'postType',
240-
'attachment',
241-
attachmentId,
242-
ATTACHMENT_EDIT_QUERY,
243-
] );
244226
return resolveAttachmentRecord( attachmentId );
245227
},
246228
[ registry, resolveAttachmentRecord ]
@@ -321,10 +303,10 @@ export function useOpenImageMediaEditorModal( {
321303
}
322304

323305
// Snapshot the attachment's current metadata before the user makes
324-
// any changes so handleMediaUpdate can compare against it later.
325-
// Prefer a freshly resolved edit-context record for accuracy; fall
326-
// back to whatever is in the cache, or a minimal object derived from
327-
// the block's own attributes when nothing is cached yet.
306+
// any changes so handleMediaUpdate can compare against it later. Use
307+
// the cached record when it's already present; only resolve when
308+
// nothing is cached yet, then fall back to a minimal object derived
309+
// from the block's own attributes.
328310
const cachedAttachmentRecord = getCachedAttachmentRecord( id );
329311
const fallbackAttachmentRecord =
330312
getAttachmentFallbackForEmptyBlockMetadata(

0 commit comments

Comments
 (0)