Skip to content

Commit 17499b1

Browse files
committed
fix(files): self-heal image dimensions from the browser instead of server-measuring
Round-3 review: server-side image-size returns raw (non-EXIF) dimensions, and clearing dims on content swap reopened the stale-PATCH race for non-image or unmeasurable content. Move authority to the browser's own naturalWidth/Height (EXIF-correct): the node view reserves from it and reports on any mismatch, and updateWorkspaceFileDimensions overwrites (no width IS NULL gate) so stale values self-correct on the next view. Reverts the server-side measurement and the content-swap dimension touch entirely.
1 parent 770c1f5 commit 17499b1

3 files changed

Lines changed: 37 additions & 51 deletions

File tree

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image.tsx

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,10 @@ function ResizableImageView({ node, updateAttributes, selected, editor }: ReactN
9494
() => source.getImageDimensions?.(attrs.src) ?? null,
9595
[source, attrs.src]
9696
)
97-
const intrinsicDimensions = storedDimensions ?? measuredDimensions
97+
// The browser's post-load measurement is authoritative — EXIF-corrected, and correct even when the
98+
// stored value is stale (e.g. left over after the file's content was replaced) — so it wins once
99+
// available; stored metadata only reserves the box pre-load. Equal in the common case, so no shift.
100+
const intrinsicDimensions = measuredDimensions ?? storedDimensions
98101
const displayWidth =
99102
dragWidth !== null
100103
? `${dragWidth}px`
@@ -134,11 +137,17 @@ function ResizableImageView({ node, updateAttributes, selected, editor }: ReactN
134137
setFailed(false)
135138
const { naturalWidth, naturalHeight } = event.currentTarget
136139
if (naturalWidth <= 0 || naturalHeight <= 0) return
137-
// Guard on the memoized `storedDimensions` the render actually uses — NOT a fresh cache read: the
138-
// memo is non-reactive, so a fresh read could see a sibling's backfill the render hasn't picked up
139-
// and skip measuring, leaving THIS view unreserved. When the render isn't reserving yet, hold the
140-
// box locally and persist (the report is idempotent, de-duped downstream).
141-
if (storedDimensions) return
140+
// The browser's measurement is authoritative. Reserve from it and persist whenever the stored
141+
// metadata is absent or disagrees (EXIF-rotated, or stale after a content swap), so a wrong value
142+
// self-corrects instead of sticking. Compare the memoized `storedDimensions` the render uses, NOT
143+
// a fresh cache read — the memo is non-reactive, and this keeps the guard consistent with render.
144+
if (
145+
storedDimensions &&
146+
storedDimensions.width === naturalWidth &&
147+
storedDimensions.height === naturalHeight
148+
) {
149+
return
150+
}
142151
setMeasuredDimensions({ width: naturalWidth, height: naturalHeight })
143152
source.reportImageDimensions?.(attrs.src, { width: naturalWidth, height: naturalHeight })
144153
}}

apps/sim/hooks/queries/workspace-files.ts

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -128,11 +128,12 @@ export function useWorkspaceFiles(
128128

129129
/**
130130
* Back the file content source's image-dimension capability with workspace file metadata. Reads intrinsic
131-
* dimensions straight from the already-loaded active file list (synchronous, so a stored image reserves
132-
* its box on the first render), and lazily backfills them once per image on first measurement. The
133-
* backfill is fire-and-forget and idempotent: the optimistic cache patch makes a second measurement of
134-
* the same file a no-op locally, and the server ignores an already-populated row — so it never storms,
135-
* never blocks render, and never touches the collaborative document.
131+
* dimensions synchronously from the already-loaded active file list (so a stored image reserves its box on
132+
* the first render), and persists the browser's measured dimensions when they're absent or disagree with
133+
* what's stored — an overwrite, so a stale value (left over after a content swap, or a non-EXIF-corrected
134+
* one) self-corrects rather than sticking. The write is fire-and-forget and de-duped (an exact-match cache
135+
* check plus mismatch-only reporting from the caller), so it never storms, never blocks render, and never
136+
* touches the collaborative document.
136137
*/
137138
export function useWorkspaceImageDimensionsAdapter(workspaceId: string): ImageDimensionsSource {
138139
const queryClient = useQueryClient()
@@ -149,13 +150,15 @@ export function useWorkspaceImageDimensionsAdapter(workspaceId: string): ImageDi
149150
},
150151
reportImageDimensions: (src, dimensions) => {
151152
const record = findRecord(src)
152-
// Skip when the file isn't ours to key (external/unlisted) or its dimensions are already stored.
153-
if (!record || (record.width != null && record.height != null)) return
154-
// Populate the cache so this and sibling views reserve space immediately and a concurrent
155-
// measurement of the same file short-circuits above. Kept even if the PATCH fails (a 403 for a
156-
// read-only member, or a transient error): the measurement is the real image size, correct
157-
// regardless of whether the write landed, so siblings should still reserve from it — a later list
158-
// refetch reconciles with the server.
153+
// Skip when the file isn't one we can key (external/unlisted), or the cache already holds exactly
154+
// these dimensions. We do NOT skip merely because SOME dimensions are stored — they may be stale
155+
// (post content-swap / EXIF), and the caller only reports the browser's authoritative measurement
156+
// on a real mismatch, so we overwrite to self-correct.
157+
if (!record || (record.width === dimensions.width && record.height === dimensions.height))
158+
return
159+
// Populate the cache so this and sibling views reserve space immediately. Kept even if the PATCH
160+
// fails (a 403 for a read-only member, or a transient error): the measurement is the real displayed
161+
// size, correct regardless of whether the write landed — a later list refetch reconciles.
159162
queryClient.setQueryData<WorkspaceFileRecord[]>(listKey, (previous) =>
160163
previous?.map((entry) => (entry.id === record.id ? { ...entry, ...dimensions } : entry))
161164
)

apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts

Lines changed: 7 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import { createLogger } from '@sim/logger'
1010
import { getErrorMessage, getPostgresConstraintName, getPostgresErrorCode } from '@sim/utils/errors'
1111
import { generateShortId } from '@sim/utils/id'
1212
import { and, eq, isNotNull, isNull, or, sql } from 'drizzle-orm'
13-
import { imageSize } from 'image-size'
1413
import type { ShareRecord } from '@/lib/api/contracts/public-shares'
1514
import {
1615
decrementStorageUsageForBillingContextInTx,
@@ -863,11 +862,12 @@ async function mapSingleWorkspaceFileRecord(
863862
}
864863

865864
/**
866-
* Backfill an image file's intrinsic pixel dimensions (a pure rendering hint used to reserve layout
867-
* space before the image loads). Idempotent by construction: the `width IS NULL` guard makes it a no-op
868-
* once populated, so concurrent first-view reporters converge on a single write with no churn. Does NOT
869-
* touch `updatedAt` — dimensions are not content and must not cache-bust the served image bytes. Returns
870-
* whether a row was actually written (false when already populated, deleted, or absent).
865+
* Store an image file's intrinsic pixel dimensions (a pure rendering hint used to reserve layout space
866+
* before the image loads). The client reports the browser's own EXIF-corrected `naturalWidth/Height`, and
867+
* only when it differs from what's stored, so this overwrites rather than backfilling once — a stale
868+
* value (e.g. left over after the file's content was replaced) self-corrects on the next view instead of
869+
* sticking behind a `width IS NULL` guard. Does NOT touch `updatedAt` — dimensions are not content and
870+
* must not cache-bust the served image bytes. Returns whether a live row was written.
871871
*/
872872
export async function updateWorkspaceFileDimensions(
873873
workspaceId: string,
@@ -881,31 +881,13 @@ export async function updateWorkspaceFileDimensions(
881881
and(
882882
eq(workspaceFiles.id, fileId),
883883
eq(workspaceFiles.workspaceId, workspaceId),
884-
isNull(workspaceFiles.deletedAt),
885-
isNull(workspaceFiles.width)
884+
isNull(workspaceFiles.deletedAt)
886885
)
887886
)
888887
.returning({ id: workspaceFiles.id })
889888
return updated.length > 0
890889
}
891890

892-
/**
893-
* Best-effort intrinsic dimensions of an image buffer (reads headers only, no full decode). Returns null
894-
* for non-images and unparseable bytes — callers then store null and let the lazy client backfill fill it.
895-
*/
896-
function measureImageDimensions(
897-
content: Buffer,
898-
contentType: string
899-
): { width: number; height: number } | null {
900-
if (!contentType.startsWith('image/')) return null
901-
try {
902-
const { width, height } = imageSize(content)
903-
return width && height ? { width, height } : null
904-
} catch {
905-
return null
906-
}
907-
}
908-
909891
/**
910892
* Look up a single active workspace file by its original name.
911893
* Returns the record if found, or null if no matching file exists.
@@ -1252,10 +1234,6 @@ export async function updateWorkspaceFileContent(
12521234
const storageBillingContext = await resolveStorageBillingContext(workspaceId)
12531235
const nextContentType = contentType || fileRecord.type
12541236
const nextStorageKey = generateWorkspaceFileKey(workspaceId, fileRecord.name)
1255-
// Re-derive intrinsic dimensions from the NEW bytes so the row never carries the previous image's size
1256-
// — and so a late fire-and-forget backfill PATCH for the OLD image (guarded on `width IS NULL`) can't
1257-
// resurrect stale dimensions, since these stay non-null for an image.
1258-
const nextDimensions = measureImageDimensions(content, nextContentType)
12591237

12601238
try {
12611239
const metadata: Record<string, string> = {
@@ -1334,10 +1312,6 @@ export async function updateWorkspaceFileContent(
13341312
key: uploadResult.key,
13351313
size: content.length,
13361314
contentType: nextContentType,
1337-
// Replaced content gets its OWN intrinsic dimensions (or null for a non-image), so the editor
1338-
// never reserves the previous image's aspect ratio and a late stale backfill can't apply.
1339-
width: nextDimensions?.width ?? null,
1340-
height: nextDimensions?.height ?? null,
13411315
updatedAt: now,
13421316
contentUpdatedAt,
13431317
})

0 commit comments

Comments
 (0)