Skip to content

Client-side media processing: only use media upload provider at the top level of the block editor provider #76124

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.

Sign up for GitHub

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

Merged
andrewserong merged 3 commits into from
Mar 5, 2026
Merged

Client-side media processing: only use media upload provider at the top level of the block editor provider #76124

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
adbccb4
Client-side media processing: only use media upload provider when not…
andrewserong Mar 4, 2026
ec1117e
Expand on comments
andrewserong Mar 4, 2026
d4227dc
Try using a key on the intercepted function instead
andrewserong Mar 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 36 additions & 4 deletions packages/block-editor/src/components/provider/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,16 +146,34 @@ export const ExperimentalBlockEditorProvider = withRegistryProvider(
const isClientSideMediaEnabled =
shouldEnableClientSideMediaProcessing();

// Nested providers (e.g. from useBlockPreview) inherit settings
// where mediaUpload has already been replaced with the
// interceptor. Detect this so we skip the replacement and
// MediaUploadProvider for them — see the longer comment below.
const isMediaUploadIntercepted =
!! _settings?.mediaUpload?.__isMediaUploadInterceptor;

const settings = useMemo( () => {
if ( isClientSideMediaEnabled && _settings?.mediaUpload ) {
if (
isClientSideMediaEnabled &&
_settings?.mediaUpload &&
Copy link
Contributor

@talldan talldan Mar 4, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part of the existing condition here is settings.mediaUpload needs to be defined for client side media processing to work. I found that curious, is this because client side media processing eventually calls through to the vanilla server version of mediaUpload?

I think an alternative fix might be to make mediaUpload undefined for BlockPreview. Users shouldn't be uploading media in previews anyway. That would avoid the need for extra isPreview checks (though would still need to do something to avoid rendering the provider, it could work the same).

Copy link
Contributor

@talldan talldan Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's where it could be implemented -

gutenberg/packages/block-editor/src/components/block-preview/index.js

Lines 59 to 66 in d37cda8

const settings = useMemo(
() => ( {
...originalSettings,
focusMode: false, // Disable "Spotlight mode".
isPreviewMode: true,
} ),
[ originalSettings ]
);

Copy link
Contributor Author

@andrewserong andrewserong Mar 4, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think an alternative fix might be to make mediaUpload undefined for BlockPreview.

Just a drive-by comment before I close my laptop for the day: setting it to undefined for BlockPreview results in the function not being available within upload-media and throws an error (as in this context, it'll wind up updating the shared settings used across the media upload provider). (I tried that approach first, but didn't get very far — I do like the idea of trying to simplify things, though 🤔 )

I think a separate fix would be to untangle the re-use of the same key (i.e. call the "real" upload function something different within upload-media), but that's a bit beyond the scope of the quick fix.

It's a little convoluted this!

Copy link
Contributor

@talldan talldan Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally none of the upload-media code executes for block previews at all, so then there would never be an opportunity to throw an error. 😄

Copy link
Contributor Author

@andrewserong andrewserong Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally none of the upload-media code executes for block previews at all, so then there would never be an opportunity to throw an error. 😄

Good point! 😄

! isMediaUploadIntercepted
) {
// Create a new object so that the original props.settings.mediaUpload is not modified.
const interceptor = mediaUpload.bind( null, registry );
interceptor.__isMediaUploadInterceptor = true;
return {
..._settings,
mediaUpload: mediaUpload.bind( null, registry ),
mediaUpload: interceptor,
};
}
return _settings;
}, [ _settings, registry, isClientSideMediaEnabled ] );
}, [
_settings,
registry,
isClientSideMediaEnabled,
isMediaUploadIntercepted,
] );

const { __experimentalUpdateSettings } = unlock(
useDispatch( blockEditorStore )
Expand Down Expand Up @@ -215,7 +233,21 @@ export const ExperimentalBlockEditorProvider = withRegistryProvider(
</SelectionContext.Provider>
);

if ( isClientSideMediaEnabled ) {
// MediaUploadProvider writes the mediaUpload function from
// _settings into the shared upload-media store so the store can
// hand files off to the server. useMediaUploadSettings extracts
// mediaUpload from the original _settings prop — *before* the
// interceptor replacement above — so the store receives the
// real server-side upload function.
//
// Only the first (outermost) provider should do this.
// Nested providers (e.g. from useBlockPreview in
// core/post-template) inherit settings that already contain
Copy link
Member

@ramonjd ramonjd Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, so would not being able to upload images on a "page" post type be due to the template it's using (in TT5)?

// the interceptor, so their MediaUploadProvider would
// overwrite the store's server-side function with the
// interceptor, causing uploads to loop instead of reaching
// the server.
if ( isClientSideMediaEnabled && ! isMediaUploadIntercepted ) {
return (
<MediaUploadProvider
settings={ mediaUploadSettings }
Expand Down
Loading