Skip to content

[WC-3477] Do not render image when the image is not available#2298

Open
r0b1n wants to merge 1 commit into
mainfrom
fix/image-loading
Open

[WC-3477] Do not render image when the image is not available#2298
r0b1n wants to merge 1 commit into
mainfrom
fix/image-loading

Conversation

@r0b1n

@r0b1n r0b1n commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Pull request type

Bug fix (non-breaking change which fixes an issue)


Description

The image widget never checked that value is not available while the image is loading, this made it start fetching images with the url /undefined. this change fixes it by not rendering the widget id the image (and fallback) is not available or still loading.

@r0b1n r0b1n requested a review from a team as a code owner July 1, 2026 14:29
@github-actions

This comment has been minimized.

@r0b1n r0b1n force-pushed the fix/image-loading branch from 5f98ebe to f1ce5f0 Compare July 1, 2026 14:43
@github-actions

This comment has been minimized.

@r0b1n r0b1n force-pushed the fix/image-loading branch from f1ce5f0 to 61babca Compare July 1, 2026 14:55
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

AI Code Review

⚠️ Approved with suggestions — low-severity items only, safe to merge


What was reviewed

File Change
packages/pluggableWidgets/image-web/src/Image.tsx Extracts getImageProps, renders null instead of a broken image element when image is falsy
packages/pluggableWidgets/image-web/src/utils/getImageProps.ts New utility — handles all three datasource types with explicit Loading state awareness
packages/pluggableWidgets/image-web/src/utils/__tests__/getImageProps.spec.ts New unit tests covering all datasource variants and status permutations
packages/pluggableWidgets/image-web/CHANGELOG.md [Unreleased] Fixed entry added
packages/pluggableWidgets/image-web/package.json Adds @mendix/widget-plugin-test-utils as a dev dependency

Skipped (out of scope): pnpm-lock.yaml

CI checks could not be retrieved — verify checks are green before merging.


Findings

⚠️ Low — loadingDynamic helper is a local workaround for a gap in test-utils

File: packages/pluggableWidgets/image-web/src/utils/__tests__/getImageProps.spec.ts lines 7–8
Note: loadingDynamic<T>() is needed because dynamic(undefined, true) actually returns Unavailable (the dynamic helper special-cases value === undefined). This is a real gap in @mendix/widget-plugin-test-utils. The local workaround is correct here, but consider contributing dynamic.loading<T>() (or a ValueStatus.Loading overload) back to the shared package so other widget tests don't need to duplicate it.


⚠️ Low — Redundant optional chaining inside status-guarded branches

File: packages/pluggableWidgets/image-web/src/utils/getImageProps.ts lines 24–25 and 32–33
Note: Inside the if (imageObject?.status === ValueStatus.Available || imageObject?.status === ValueStatus.Loading) guard, imageObject is guaranteed non-null, so imageObject?.value?.uri has a redundant first ?.. Same for defaultImage?.value?.uri in the next branch. The second ?.value?.uri is correct (value can be undefined while loading), but the outermost ?. after the object is a no-op. Not harmful, just imprecise.

// current
image: imageObject?.value?.uri

// clearer
image: imageObject.value?.uri

Positives

  • The root fix is precise and minimal: adding return image ? (...) : null in Image.tsx directly solves the /undefined network request without touching unrelated rendering paths.
  • Extracting getImageProps into its own module makes the loading-state logic independently testable — a clear improvement over the inline function that was impossible to unit-test directly.
  • Test coverage is thorough: every status combination (Available, Loading-with-value, Loading-without-value, Unavailable, missing prop) is exercised across all three datasource modes.
  • Uses @mendix/widget-plugin-test-utils (dynamic) correctly and avoids manual mock objects.
  • CHANGELOG entry is user-facing language (describes the visible symptom), not implementation detail — exactly right.

@r0b1n r0b1n changed the title [WC3477] Do not render image when the image is not available [WC-3477] Do not render image when the image is not available Jul 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant