Skip to content

fix: omit empty optional image fields#7167

Open
ahfoysal wants to merge 2 commits into
tinacms:mainfrom
ahfoysal:fix-empty-optional-image-fields
Open

fix: omit empty optional image fields#7167
ahfoysal wants to merge 2 commits into
tinacms:mainfrom
ahfoysal:fix-empty-optional-image-fields

Conversation

@ahfoysal

@ahfoysal ahfoysal commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Fixes #5229.

Omit empty optional image fields from document payloads so blank image fields are not written to frontmatter.

Tests:

  • pnpm --filter @tinacms/graphql exec vitest run src/resolver/index.test.ts --coverage.enabled
  • pnpm exec biome check packages/@tinacms/graphql/src/resolver/index.ts packages/@tinacms/graphql/src/resolver/index.test.ts
  • pnpm turbo run build --filter=@tinacms/graphql

@ahfoysal
ahfoysal requested a review from a team as a code owner July 7, 2026 05:54
@changeset-bot

changeset-bot Bot commented Jul 7, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: ece9c05

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 15 packages
Name Type
@tinacms/graphql Patch
@tinacms/cli Patch
@tinacms/datalayer Patch
@tinacms/search Patch
playwright-testing Patch
tinacms-authjs Patch
tinacms-gitprovider-github Patch
tinacms Patch
next-tinacms-azure Patch
next-tinacms-cloudinary Patch
next-tinacms-dos Patch
next-tinacms-s3 Patch
tinacms-clerk Patch
@tinacms/app Patch
@tinacms/vercel-previews Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@joshbermanssw joshbermanssw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Requesting changes for the update clear case.

throw new Error(`Expected to find field by name ${fieldName}`);
}
if (isEmptyOptionalImageValue(fieldValue, field)) {
continue;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Requesting changes here: this handles a blank optional image by omitting the field from the mutation values, but update persistence later merges those values back onto the existing document ({ ...legacyValues, ...params } in resolveUpdateDocument() and { ...oldDoc, ...values } in the deprecated update path). That means an existing coverImage: "foo.png" followed by an editor submit of coverImage: "" can produce empty mutation values for this field and preserve the old image instead of clearing it from frontmatter. Please add coverage for clearing an existing optional image on update and make the explicit clear remove the key from the merged payload, while keeping truly omitted fields as unchanged.

@ahfoysal

Copy link
Copy Markdown
Contributor Author

Fixed the update clear case. Empty optional image values are still omitted on create, but now clear an existing image on update. Added a resolver regression test for that path.

};
break;
case 'image':
if (isEmptyOptionalImageValue(value, field)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Question on this one: why change the read path too?

The frontmatter bug only needs the write path. This makes an existing document with avatar: '' return null instead of "" over GraphQL, which is an API-visible change for existing content shipping as a patch.

If it's load-bearing for something I've missed, could you note it in the changeset? If not, dropping it would make the PR strictly safer.

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.

🤖 Bug - optional image field adds frontmatter key even when no media assigned

2 participants