Skip to content

fix(RemoteAsset): ignore base64 data URI images#1222

Open
andershagbard wants to merge 3 commits into
Shopify:mainfrom
andershagbard:fix/remote-asset-ignore-base64-images
Open

fix(RemoteAsset): ignore base64 data URI images#1222
andershagbard wants to merge 3 commits into
Shopify:mainfrom
andershagbard:fix/remote-asset-ignore-base64-images

Conversation

@andershagbard
Copy link
Copy Markdown
Contributor

Summary

  • The RemoteAsset check was incorrectly flagging src attributes containing base64-encoded data URIs (e.g. src="data:image/gif;base64,...") as needing to be served via the Shopify CDN.
  • Data URIs are fully self-contained — they embed the image bytes directly in the attribute value and create no external HTTP connections, so the check's performance concern does not apply.
  • Added an isDataUri helper (mirrors the existing isHashUrl helper) and an early-return guard in checkHtmlNode so data URIs are silently skipped.

Example

{{! Before: this was incorrectly reported as an offense }}
<img
  src="data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw=="
/>

Test plan

  • should not report an offense for base64 encoded data URIs — single GIF data URI passes
  • should not report an offense for various base64 data URI image formats — PNG, JPEG, SVG, WebP data URIs all pass
  • should still report an offense for remote images alongside base64 images — mixed case: base64 is ignored, remote URL is still caught
  • All 22 tests in remote-asset/index.spec.ts pass

🤖 Generated with Claude Code

Data URIs (e.g. `src="data:image/gif;base64,..."`) are self-contained
and do not create external HTTP connections, so the RemoteAsset check
should not flag them.

Adds an `isDataUri` helper and an early-return guard in `checkHtmlNode`
alongside the existing hash-URL guard.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@andershagbard andershagbard requested a review from a team as a code owner June 2, 2026 10:42
Copy link
Copy Markdown
Contributor

@graygilmore graygilmore left a comment

Choose a reason for hiding this comment

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

Love this! Two small tweaks suggested.

Comment thread packages/theme-check-common/src/checks/remote-asset/index.ts Outdated
Comment thread packages/theme-check-common/src/checks/remote-asset/index.ts
andershagbard and others added 2 commits June 3, 2026 08:19
Co-authored-by: Gray Gilmore <graygilmore@gmail.com>
Handles `{{ 'data:image/gif;base64,...' | img_tag }}` patterns the same
way HTML src attributes with data URIs are already handled.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@andershagbard andershagbard requested a review from graygilmore June 3, 2026 10:40
Copy link
Copy Markdown
Contributor

@graygilmore graygilmore left a comment

Choose a reason for hiding this comment

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

Thanks @andershagbard! Just needs a changeset which you can create with pnpm changeset add. A patch is fine for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants