Repository navigation
feat!: add file transform state - #18601
nathanlentz wants to merge 6 commits into
Conversation
📦 esbuild Bundle Analysis for payloadThis analysis was generated by esbuild-bundle-analyzer. 🤖
Largest pathsThese visualization shows top 20 largest paths in the bundle.Meta file: packages/next/meta_index.json, Out file: esbuild/index.js
Meta file: packages/payload/meta_index.json, Out file: esbuild/index.js
Meta file: packages/payload/meta_shared.json, Out file: esbuild/exports/shared.js
Meta file: packages/richtext-lexical/meta_client.json, Out file: esbuild/exports/client_optimized/index.js
Meta file: packages/ui/meta_client.json, Out file: esbuild/exports/client_optimized/index.js
Meta file: packages/ui/meta_shared.json, Out file: esbuild/exports/shared_optimized/index.js
DetailsNext to the size is how much the size has increased or decreased compared with the base branch of this PR.
|
ce8c967 to
bf5a4f1
Compare
bf5a4f1 to
eb62f07
Compare
| let replayPipeline: PlannedTransformer[] | undefined | ||
| let replaySource: FileSource | undefined | ||
|
|
||
| if (!file && retainedOriginal && transformStateWrite.hasChanged) { |
There was a problem hiding this comment.
Replay transforms for legacy uploads
Replay requires retainedOriginal, but legacy local records do not have an original object. An update can save _transforms without updating the file or its variants. Normalise legacy uploads before generation, or use the verified legacy file as the retained source. Add single and bulk update tests.
There was a problem hiding this comment.
Legacy uploads now use the verified existing file as the original. Added single and bulk update tests, including a missing-file case.
| const result = await transformer.handleRequest({ | ||
| collectionSlug: collection.config.slug, | ||
| doc, | ||
| getOriginalFile: async () => { |
There was a problem hiding this comment.
Give each transformer its own original response
The request uses one single-use original getter for every stage. Persisted processing can consume it before a later stage calls getOriginalFile. The source and original also share one abort controller, which can close or leak a response body. Give each stage a fresh getter and track each response with its own controller.
There was a problem hiding this comment.
Fixed. Each stage gets its own original response and cleanup handling. Added tests for shared streams, failures, and cancellation.
| if (state.rotate) { | ||
| output = output.rotate(((state.rotate.angle % 360) + 360) % 360) | ||
| } | ||
| if (state.resize) { |
There was a problem hiding this comment.
Limit persisted resize dimensions
API-controlled _transforms.resize values reach Sharp without width, height, pixel, or frame limits. A permitted update can cause excessive CPU or memory use during replay or request processing. Apply configurable limits before both resize paths, including derived dimensions and animation frames.
There was a problem hiding this comment.
I added configurable limits for uploads and requests, including calculated sizes and all animation frames. Defaults are 4096px per side and about 16.8 million total pixels.
| const dimensions = await sharpDependency(rotated, constructorOptions).metadata() | ||
| const requestedScale = Math.max( | ||
| state.resize.width / dimensions.width, | ||
| state.resize.height / dimensions.height, |
There was a problem hiding this comment.
Use the height of each animation frame
Sharp reports the total stacked height for animated images and provides the frame height as pageHeight. This calculation creates an oversized intermediate image and an incorrect crop. Use metadata.pageHeight ?? metadata.height for per-frame calculations. Add animated focal-point pixel and frame-count tests.
There was a problem hiding this comment.
Fixed to use pageHeight when available. Added a test that checks the crop in both frames and preserves frame count and timing.
| path: (number | string)[] | ||
| previous: Document | ||
| }): Promise<void> { | ||
| if (!data || typeof data !== 'object') { |
There was a problem hiding this comment.
Validate required fields inside removed containers
This return stops validation when a transformer removes a group or named tab. Required descendant fields are not checked, so invalid documents can be saved. Continue with an empty current object and force descendant validation when an ancestor changes.
There was a problem hiding this comment.
Fixed. Removing a group or named tab now checks its required fields. Added tests to make sure invalid changes aren’t saved.
| return | ||
| } | ||
| if (isNewFile) { | ||
| setTransforms(null) |
There was a problem hiding this comment.
Keep transforms when replacement is cancelled
Entering replacement mode clears _transforms, but Cancel only hides the replacement controls. Saving another field then removes crop, focal-point, and custom transform data. Clear transforms after a replacement file is selected, or restore the previous state on Cancel. Extend the test to save and verify _transforms.
There was a problem hiding this comment.
Cancel now keeps the pending transforms. The test also saves another field afterward and checks that all transform keys survive.
| const write = () => updateDocument(updateArgs) | ||
| const prepared = await prepareUpdateDocument(updateArgs) | ||
| generatedFileData = collectionConfig.upload | ||
| ? await generateFileData({ |
There was a problem hiding this comment.
Pass the bulk update access mode to validation
This call omits overrideAccess, so final validation always receives false. A trusted bulk Local API update can fail when an equivalent single-document update succeeds. Pass overrideAccess and add a parity test.
There was a problem hiding this comment.
Fixed....should be required. /s
| closeModal(editDrawerSlug) | ||
| } | ||
|
|
||
| const onDragEnd = React.useCallback(({ x, y }) => { |
There was a problem hiding this comment.
Clear errors after correction through alternate controls
Dragging or using arrow keys updates the focal point without clearing its input errors. The crop control has the same problem. Valid values can still show an alert and keep Apply disabled. Revalidate or clear the related errors, then test drag and keyboard correction.
There was a problem hiding this comment.
Fixed. Dragging, arrow keys, and reset now clear the errors they correct. Added coverage for crop and focal controls.
|
Open to supporting legacy to make this additive for now if we cannot afford the breaking change in beta. |
paulpopus
left a comment
paulpopus
left a comment
There was a problem hiding this comment.
Written with AI
The current E2E failures reproduce the unnamed filter option and animated WebP encoding changes in both app frameworks.
|
|
||
| const sizeResultFile = await transform({ | ||
| fieldPath, | ||
| file: mainResultFile, |
There was a problem hiding this comment.
Avoid a second lossy encoding pass for variants
Each variant now reads the encoded main result. Animated WebP files are encoded again, and both upload E2E jobs produce 200380 bytes instead of 211638. Compose the main and variant operations from the retained original, or use a lossless intermediate. Keep a regression test for output quality.
There was a problem hiding this comment.
Variants now use bytes from original, keeping the main image's edits
| const { data: newFileData, files: filesToUpload } = await generateFileData({ | ||
| collection, | ||
| config, | ||
| data = await prepareUploadData({ |
There was a problem hiding this comment.
Keep final upload metadata available to hooks
prepareUploadData gives hooks source metadata, while generateFileData now runs after every before-hook. Hooks that derive fields from dimensions, MIME, filename, or filesize can store values that disagree with the final file. Preserve the former contract, or document the breaking change and provide a migration path with focused tests.
There was a problem hiding this comment.
To achieve this we are keeping hooks before transforms so they can set _transforms. I elevated the symptom to metadata in the breaking changes of the PR.
| // need the whole file, so leave such an upload untouched rather than buffering it. | ||
| const canRunTransformers = pipeline.length > 0 && hasFullFileContents(file) | ||
| assertTransformCoverage({ pipeline: requestPipeline, state: workingDoc._transforms }) | ||
| expectedDefaultMimeType = workingDoc.mimeType |
There was a problem hiding this comment.
Persist the effective identity for request-only conversions
This code copies the source MIME and derives the logical filename from the original extension. A request-only PNG-to-WebP conversion then stores PNG metadata while returning image/webp. Let adapters declare the output identity, or store unknown metadata as null and make routing support it. Add a request-only conversion test.
There was a problem hiding this comment.
Went with null for output metadata we don't know yet and ditched the extension from logical default filenames.
| typeof value !== 'object' || | ||
| seen.has(value) || | ||
| Object.getOwnPropertySymbols(value).length > 0 || | ||
| (Array.isArray(value) && Object.keys(value).length !== value.length) || |
There was a problem hiding this comment.
Reject arrays that change during JSON serialisation
This key-count check accepts a sparse array when it also has an extra enumerable property. JSON then fills the hole with null and removes the extra property, so replay receives a different value. Verify that every array key is its canonical numeric index and that every index is present. Add tests for sparse arrays and custom properties.
There was a problem hiding this comment.
This has been buttoned up.
This branch has not been deployed
2 participants
Uploads can now save crop, focal point, and other edits in
_transformswhile keeping the original file. Change or clear those edits later and regenerate the output from the original. How neat!null, and{}also becomesnull.Upload edit API: write crop and focal edits through
_transforms. Legacy crop/focal upload-edit query writes are removed;focalXandfocalYare read-only compatibility fields.Upload hooks:
beforeValidateandbeforeChangenow run before transforms and see source metadata on new or replacement uploads. Move calculations that need processed bytes into a transformer; useafterChangefor final filenames.Dynamic defaults: output metadata stays
nulluntil delivery, and logical default filenames have no format extension. Use the responseContent-Typefor the output format.Custom transformers: update adapters to the new document/source contract.
transformFilereceivessourceandoriginalSourceinstead offile. Adapters must claim saved keys throughhandledTransformKeys.Sharp variants: variants derive from the transformed default image. Larger variants may be skipped when enlargement is disabled.
Version restoration: saved edits replay through the current adapters, so regenerated bytes may differ from historical output.
_transformsreplaces the whole state. Omit it to keep saved edits, or sendnullto clear them. Replacing the original without supplying state clears the previous edits.