Conversation
github-actions
bot
added
[Package] Editor
adamsilverstein
added
[Type] Enhancement
github-actions
bot
added
[Package] Editor
github-project-automation
bot
moved this to 🔎 Needs Review
in WordPress 7.0 Editor Tasks
github-actions
bot
added
[Package] Element
|
I merged #74951 into trunk and then trunk into this branch. In my local build this seems to have fixed the bundle size. I assume the CI will run and update the PR warning. Claude also suggested lazy loading @wordpress/vips which I am investigating now. Makes sense to only load vips when we need it. |
The vips package generates a 10MB+ worker-code.ts file during builds that crashes ESLint's text-table formatter. The file is already in .gitignore.
The @wordpress/vips/worker module contains ~10MB of inlined WASM code. Loading it eagerly at module parse time adds unnecessary overhead when no image processing occurs. This defers the import to the first actual vips function call and caches the module for subsequent use.
|
I added lazy loading of vips in df87386 which should help with load speed and memory usage. Image processing continued to work well after this commit in my testing. Assuming all linters and tests pass in CI, this PR will be ready for review again. |
The block-library package depends on @wordpress/upload-media but the tsconfig reference was missing, causing the lint:tsconfig CI check to fail.
|
@andrewserong @ramonjd - this is ready for another review. I did some basic testing to verify the image processing still works as expected and the CI bundle size warning was solved by unbundling from the other PR. |
|
Thanks for the ping @adamsilverstein this is looking closer. I think there might still need to be some configuration changes as the automated build comment is saying that 
To test, I'm switching the client-side media experiment off and doing a hard refresh of the post editor. In this case, the Something I'm wondering:
Please let me know if there's anything else you'd like us to test or if you were seeing something different in your testing! |
Thanks for the links and continues testing. I thought the previous unbundling work had changed the over sized warning. Testing with the experiment off and checking the network panel are good debugging tips. I'll take a look at the links you shared and continue iterating to get that load size down. There will of course be some weight to the feature overall as it's adding a significant library and capabilities. That said, these should not load if the experiment is off and more specifically until the feature is actually used - eg not when the editor loads, but rather when a user goes to upload media for the first time. |
It did! It just also moved where the over size was happening, but I think we're getting closer 😄
Totally. I think the lazy loading will be key to mitigating the weight of the feature. One nuance (which is why I think it's so important to get right) is that the client side media processing is weighty both in bytes loaded and computational complexity, so getting the lazy loading in place will help us for the feature gating. I.e.
So, all up I think this part of the work will very much be worth it. Thanks again for all the back and forth on it! |
|
Sorry I haven't had time to test this lately, trying to ship a bunch of other things for 7.0 atm. Thanks for the back and forth, folks! |
The upload-media IIFE bundle was 3.81MB because vips (with inlined WASM) was bundled inline. Adding wpScriptModuleExports to the vips package lets the build system externalize it, producing a separate on-demand script module. Upload-media drops to ~19KB; vips loads only when image processing runs.
|
@andrewserong - I think db4bd7e addresses your recent feedback, can you please give it a test? |
…d-images # Conflicts: # packages/editor/src/components/provider/use-block-editor-settings.js
andrewserong
left a comment
andrewserong
left a comment
There was a problem hiding this comment.
Nice work, the lazy loading seems to be working perfectly to me!
With the client-side experiment switched off, the upload-media package is nice and small:
And what gets downloaded from vips (the loader) is < 1kb:
Uploading (with the experiment off) doesn't load the rest of vips, as expected.
Then, with the client-side experiment switched on, the above holds true, but when I go to upload my first file, the worker gets loaded (while the spinner happens in the block editor UI). This is then when the 3.8mb gets downloaded:
Subsequent uploads don't re-download / import anything. I'd say the lazy loading is working just as intended now 🎉
|
@andrewserong shall we merge this or await further review? Once merged, we will have a functioning Client Side Media experiment and the remaining PRs should be much smaller and hence easier to review as well as actually test. |
andrewserong
left a comment
andrewserong
left a comment
There was a problem hiding this comment.
shall we merge this or await further review? Once merged, we will have a functioning Client Side Media experiment and the remaining PRs should be much smaller and hence easier to review as well as actually test.
This is testing great for me now (even when uploading 33 images at once I'm not seeing any failures), and the lazy loading is appropriately guarded behind the experiment. So IMO for all the important things we were concerned about with such a large PR, those are all addressed, so I'll give this a tentative approval. I don't feel like I've thoroughly tested (or understood) the feature, but I think it's been appropriately guarded and heading in the right direction 👍
The main areas I feel slightly tentative about are:
- The naming / inclusion of the additional block editor settings keys (
allImageSizes,bigImageSizeThreshold) — these seem fine to me, but they can be hard (or impossible) to remove once added, so just thought I'd mention them in case anyone else has opinions on it - The changes to the
wp-buildpackage specifically forvips— this also seems okay to me, but it isn't general purpose, and is a bit tied to this particular case. I'll just ping @youknowriad on that one in case it's of any concern
Overall, I'd love to see this PR land as I think it'll help unblock the subsequent and smaller PRs to polish this feature. That said, given that I feel slightly tentative about it, I've added gutenberg-core as reviewers just so other folks can have visibility on this as it's a substantial PR we're intending to land here.
Still, once in trunk, it should be fairly quick to ship follow-ups, so this PR has my vote. Thanks again for all the discussion and iterations!
| // Registered image sizes from the server. | ||
| allImageSizes?: Record< | ||
| string, | ||
| { width: number; height: number; crop: boolean } |
There was a problem hiding this comment.
Claude's telling me that crop can also be an array, which seems legit from looking at the add_image_size function (i.e. it could be [ 'left', 'top' ])
There was a problem hiding this comment.
I'll look into that, thanks.
|
Thanks for the continued support and tentative approval! I will give others some time to weigh in with any additional feedback or reservations. |
There was a problem hiding this comment.
I also took it for another smoke test. Working pretty well, thanks a lot for the effort here. 🍺
Agree that it'd be good to give other folks the chance to chime in before merging.
Detect (somehow) that we're on a low powered device and skip client-side media processing and downloading the additional JS altogether (I think you might have mentioned this already elsewhere)
Good area to explore for later. That or if vips fails to load, an explicit fallback to server-side generation.
In general, I think the current design of loading on first actual upload works for decent connections.
When throttling to 3G uploading 15 x <10kb images, there was almost a 60sec wait to download worker.js Here's a sneak preview 😄
Kapture.2026-02-13.at.15.46.02.mp4
I only added the screencast because it reminded me that we could reduce user anxiety by adding progress reporting in the block editor. It's a later thing.
| }, | ||
| additionalData: { | ||
| convert_format: false, | ||
| generate_sub_sizes: false, |
There was a problem hiding this comment.
This will be passed for every addItem call, regardless of whether the experiment is enabled, right?
Not sure if it matters, just asking 😄
There was a problem hiding this comment.
yes, for every images added to the upload queue (AddAction). This is expected, these are images that will be processed then sideloaded.
| * | ||
| * @return {Object} The esbuild plugin. | ||
| */ | ||
| function createWasmVipsCommonJsPlugin() { |
There was a problem hiding this comment.
Can you clarify why we need this? I'm also wondering how we can make this generic? It's important to note that wp-build is not a gutenberg only build tool so ideally exceptions like that are avoided if we can.
There was a problem hiding this comment.
Thanks for reviewing and for pointing out WP-build is not meant to be GB specific.
I believe this was added to ensure we get the commonjs version of wasm-vips instead of the module version. The module version caused issues when run as bundled / encoded base64. I will double check to verify my memory, add some inline docs at a minimum and also investigate a way to make this more generic or possibly externalize the bit we need.
There was a problem hiding this comment.
this was added in 65e5893
in summary, because module vips uses import.meta.url which in Blob URL Worker context is relative to the blob and doesn't work.
Claude summarized the reasoning as:
This plugin redirects esbuild from wasm-vips's ES module entry (vips-es6.js) to its CommonJS entry (vips.js) because the ESM version uses import.meta.url in three places (base path resolution, WASM file location, and pthread worker spawning), which all break when the bundled worker code runs in a Blob URL context since import.meta.url resolves to an invalid blob: URL instead of a real file path
I'll work on your other feedback next.
There was a problem hiding this comment.
@youknowriad here is what i came up with: f42a103 - this adds a more generic capability to wp-build for handling just the type of mapping we need here.
We still use the same approach, making it generic so any package could use the capability...
The changes required for mapping vips bundling to the file we need are now made in the vips package.json file keeping the concern where it belongs
The plugin is updated and now enables redirecting module loads for any package...
what do you think?
There was a problem hiding this comment.
Where can I read about the need to swap imports in the first place. I mean we're in a build script for a worker, why can 't we just tell the build tool that: "you're in a worker, if you encounter an import that is esmodule, transpile it to CJS". Isn't that possible?
There was a problem hiding this comment.
I also asked Claude to explain in more detail and got this:
Good question. The reason we can't just tell esbuild "transpile ESM to CJS" is that the worker bundle itself needs to remain ESM — the worker is loaded via new Worker(blobUrl, { type: 'module' }) in vips-worker.ts, so it must be ESM format.
When esbuild outputs format: 'esm', it preserves import.meta.url as-is (there's no option to transform it). Switching to format: 'cjs' would transpile import.meta.url away, but the resulting CJS bundle can't be loaded as a type: 'module' worker.
So the redirect is a targeted fix: we swap just the problematic dependency (vips-es6.js → vips.js) while keeping the overall worker bundle as ESM. The CJS version of wasm-vips uses _scriptName/currentScript.src instead of import.meta.url, which works fine when bundled.
An alternative would be to drop type: 'module' from the Worker constructor and build as IIFE format — that would eliminate the redirect entirely. But it would also prevent use of top-level await and other ESM-only features in worker code. The redirect approach felt like the less restrictive trade-off.
I guess the question is whether the embedded blob url is something we'll be using for all workers in WordPress or not
regarding why we use base64 in the first place for wasm:
this is documented in packages/wp-build/lib/build.mjs:229-230:
Serving as base64 eliminates the need for separate WASM file downloads and avoids issues with hosts not serving WASM files with the correct MIME type. This is critical because WebAssembly.compileStreaming() requires the response to have Content-Type: application/wasm - and many WordPress hosts may not have that MIME type configured for .wasm files, which would cause a hard failure. Hence, wasmInlinePlugin converts .wasm files to data:application/wasm;base64,… data URLs at build time.
There was a problem hiding this comment.
I think it's ok to ship as is now.
🎉 Thanks for the review @youknowriad! I'll wait to see if @jsnajdr has any additional feedback on the PR, then get this merged so I can continue working on the other smaller issues (which are in progress and mostly need rebasing) as well as updating the core backport PR.
There was a problem hiding this comment.
To be honest, I'm still lost when it comes to workers ... and the way we're using them here.
The main thing we leverage workers for here is running the relatively heavy image processing itself in the background (off the main thread), which keeps the editor operating smoothly. Without it, the editor would become unresponsive during uploads. Running code in workers does add layers of additional complexity and restrictions. The payoff in the end is worth it though and I'm sure we will find other uses for this - any time we have a complex task to complete that could cause jank by bogging down the main thread.
There was a problem hiding this comment.
question is whether the embedded blob url is something we'll be using for all workers in WordPress or not.
The blob is strictly necessary only for WASM code. Here some hostings block .wasm files, so they can't be loaded directly. We need to wrap them in a JS module and load as a blob literal.
The worker script itself could be a .js file. We don't really need the indirection where:
const workerCode = "string(); with() + JS.code";
new Worker( URL.createObjectURL( new Blob( workerCode ) ) );The code can be in its own .js file, it doesn't need to be a string.
Something we can improve, but not a blocker for this specific PR.
When esbuild outputs format: 'esm', it preserves import.meta.url as-is (there's no option to transform it). Switching to format: 'cjs' would transpile import.meta.url away, but the resulting CJS bundle can't be loaded as a type: 'module' worker.
I think we could build the worker scripts with format: 'iife'. That corresponds best with what the worker script really contains. It's a self-contained script that doesn't export anything and bundles all imports. So there is no module code left, except that failing import.meta.url statement.
In CJS, the import.meta.url is compiled as self.location.url, and in IIFE it's import_meta = {}; import_meta.url. That means undefined.
If we build the worker script as a normal file and use blobs only for WASM, then the import.meta.url problem should go away: i) import.meta.url is a valid syntax inside an ES module; ii) once the script URL is a normal URL, then you can construct relative URLs with new URL( '.', import.meta.url ), something that's not possible with blob URLs.
Also, building both esm and cjs formats of the worker script, depending on package.json fields, is not neccessary at all. The code that loads the output is new Worker( url, { type: 'module' } ), so all we need is an ESM build, nothing else.
There was a problem hiding this comment.
One additional point about import.meta.url code in the wasm-vips library: we probably never use it at all. It's used in the code that looks up the .wasm module, and we override that completely with our own locateFile callback. The default fallback implementation is not used.
The hardcoded createWasmVipsCommonJsPlugin in wp-build contained package-specific knowledge about wasm-vips. This moves the resolve config into the vips package's wpWorkers declaration and replaces the plugin with a generic createModuleRedirectPlugin that reads from the new config. Also documents the wpWorkers config format.
The inline type had crop typed as just boolean, but WordPress's add_image_size() also accepts a positional array like ['left', 'top']. Reuse the existing ImageSizeCrop interface which already has the correct type.
| vipsRotateImage, | ||
| vipsCancelOperations, | ||
| terminateVipsWorker, | ||
| } from './vips'; |
There was a problem hiding this comment.
Why do we need the index.ts file when we could just rename vips.ts to index.ts?
| "@wordpress/preferences": "file:../preferences", | ||
| "@wordpress/private-apis": "file:../private-apis", | ||
| "@wordpress/url": "file:../url", | ||
| "@wordpress/vips": "1.0.0-prerelease", |
There was a problem hiding this comment.
This should be a file:../vips reference, it's a package in the same monorepo.
There was a problem hiding this comment.
Yes, it will break the published package.
There was a problem hiding this comment.
pnpm would not let you merge this PR as the package doesn't exist on the npm repository yet.
jsnajdr
left a comment
jsnajdr
left a comment
There was a problem hiding this comment.
I think this is mergeable. There are many opportunities to improve and refine our worker/WASM setup, but that's a topic for multiple future PRs.
|
Thanks all for the review and support here! @jsnajdr I'm going to merge what we have here, then open a follow up Issue to address your most recent feedback items. |
6 participants
What?
Closes #74355
Depends on #74352
Add client-side thumbnail generation for uploaded images in the
@wordpress/upload-mediapackage. When an image is uploaded, this PR enables the browser to generate sub-sized images (thumbnails) using the vips library in a web worker, then sideloads them to the server.Why?
By generating thumbnails client-side using WASM (via the
@wordpress/vipspackage), we can:How?
This PR introduces several key changes to the
@wordpress/upload-mediapackage:Vips utility wrappers (
packages/upload-media/src/store/utils/vips.ts):Thin wrappers around
@wordpress/vips/workerfunctions that handle File/Blob conversions:vipsResizeImage: Resizes images to specified dimensions with optional cropping and dimension suffixvipsConvertImageFormat: Converts images between formats (JPEG, PNG, WebP, AVIF, GIF)vipsCompressImage: Compresses images with quality controlvipsHasTransparency: Checks if an image has transparency (with error handling fallback)vipsRotateImage: Rotates images based on EXIF orientation valuesvipsCancelOperations: Cancels ongoing vips operations for a given queue itemNew operation types (
OperationTypeenum):ResizeCrop: Resizes and crops images to specific dimensions viaresizeCropItemactionThumbnailGeneration: Orchestrates creating multiple thumbnail sizes viagenerateThumbnailsactionRotate: Handles EXIF-based image rotation viarotateItemactionBig image size threshold scaling:
bigImageSizeThresholdsetting from REST API (default 2560px) through block editor settings-scaledsuffix to scaled images, matching WordPress core'swp_create_image_subsizes()behaviorAutomatic image rotation:
exif_orientationfrom REST API whengenerate_sub_sizes=false-rotatedversion sideloadedSideloading support:
addSideloadItemaction: Queues a file for sideloading (typically a generated thumbnail)sideloadItemaction: Uploads generated thumbnails to existing attachments viamediaSideloadSideloadMediaArgsinterface: Defines the contract for sideload operationsSideloadAdditionalDatainterface: Includespost(attachment ID) andimage_sizenameImage size configuration:
allImageSizesto Settings type: Maps size names to{ width, height, crop }definitionsQueue management improvements:
isUploadingByParentIdselector: Tracks child uploads to prevent premature parent removalisUploadingToPostselector: Detects concurrent uploads to the same attachmentgetPausedUploadForPostselector: Finds paused items for a given post/attachmentshouldPauseForSideloadhelper: Pauses uploads to avoid race conditions when sideloadingresumeItemByPostIdaction: Resumes paused uploads after sideload completesmissing_image_sizesfrom attachment response to determine which sizes need generationTesting Instructions
Prerequisites / Setup
Enable the Client Side Media experiment
Prepare test images (for comprehensive testing):
Open browser DevTools → Network tab to observe upload behavior
Feature 1: Basic Client-Side Thumbnail Generation
What it does: When an image is uploaded, the browser generates sub-sized images (thumbnails) using the vips library, then sideloads them to the server.
Testing steps:
/wp/v2/media)image_sizeparameter)Feature 2: Big Image Size Threshold Scaling
What it does: Images larger than 2560px (configurable via
big_image_size_thresholdfilter) are automatically scaled down before upload, with a-scaledsuffix added. Note one bug: currently the original image is not uploaded when a scaled image is created, see #75320.Testing steps:
-scaledin its filename/wp-json/wp/v2/media/{id})big_image_size_thresholdfilter to adjust the threshold and test againEdge cases to test:
Feature 3: Automatic Image Rotation (EXIF)
What it does: Images with EXIF orientation data are automatically rotated to display correctly, matching WordPress core's server-side behavior.
Testing steps:
For images smaller than threshold that need rotation:
-rotatedsuffix version is created and sideloadedEdge case:
Feature 4: Upload Cancellation
What it does: Canceling an upload mid-process properly cleans up all related operations including thumbnail generation.
Testing steps:
Feature 5: Custom Image Sizes (Theme/Plugin)
What it does: Custom image sizes registered by themes or plugins are also generated client-side.
Testing steps:
functions.php:Feature 6: Concurrent Upload Handling
What it does: Multiple images can be uploaded simultaneously without conflicts.
Testing steps:
Error Handling (Optional)
Network failure during sideload:
Notes for Reviewers
POSTrequests per image uploadTechnical Notes
Dependencies added:
@wordpress/vips: WebAssembly-based image processing library- see Load vips library with new wordpress/worker-threads #74785New/Updated Types:
ImageSizeCrop: Defines width, height, and crop settings for image sizes (supports positional crop anchors)SideloadAdditionalData: Additional data for sideload requests (attachment ID, image size name)SideloadMediaArgs: Arguments for themediaSideloadfunctionSettings.allImageSizes: Registry of available image sizes from the serverSettings.bigImageSizeThreshold: Threshold for automatic image scaling (default 2560)Test coverage:
isUploadingByParentIdselector