Skip to content

Skip None fields when serializing generated input objects#227

Open
achitojha wants to merge 3 commits into
mainfrom
05-18-skip_none_fields_when_serializing_generated_input_objects
Open

Skip None fields when serializing generated input objects#227
achitojha wants to merge 3 commits into
mainfrom
05-18-skip_none_fields_when_serializing_generated_input_objects

Conversation

@achitojha
Copy link
Copy Markdown

@achitojha achitojha commented May 19, 2026

Issue : https://github.com/shop/issues-shopifyvm/issues/318.

This updates generated input-object serialization so generated Option<T> fields that are None are omitted instead of serialized as explicit null values.

Tests

  • cargo fmt --check
  • cargo test
  • cargo clippy --all-targets -- -D warnings

@achitojha achitojha requested review from adampetro and Copilot and removed request for adampetro May 19, 2026 04:56
@achitojha achitojha marked this pull request as ready for review May 19, 2026 04:57
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR changes the generated Serialize implementation for GraphQL input objects so that optional fields (those generated as Option<T>) are omitted from the serialized output when None, rather than being written as explicit null values. Previously every field was written unconditionally, which led to unwanted explicit nulls in the wasm-API output buffer.

Changes:

  • In ShopifyFunctionCodeGenerator::additional_impls_for_input_object, split field emission by is_required(): required fields are always written, optional fields are wrapped in an if let Some(value) = ... and only written when present.
  • Compute the object's field count dynamically as num_required_fields + Σ usize::from(self.<opt>.is_some()) so write_object receives the correct size.
  • Add a SerializationProbe input to the example schema and three tests covering omission of None optional fields, inclusion of Some fields, and @oneOf input object serialization.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
shopify_function_macro/src/lib.rs Generates conditional emission of optional input-object fields and computes the field count from required + present-optional counts.
example_with_targets/src/tests.rs Adds tests verifying that None fields are omitted, Some fields are included, and @oneOf variants serialize correctly.
example_with_targets/schema.graphql Adds SerializationProbe input with a mix of optional, defaulted, and required fields to support the new tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

@adampetro adampetro left a comment

Choose a reason for hiding this comment

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

Can we do some validation on how this affects instruction count in the worst case (when we don't skip serialization but have some extra checks to do)?

Comment thread shopify_function_macro/src/lib.rs Outdated
Comment thread shopify_function_macro/src/lib.rs Outdated
achitojha and others added 2 commits May 19, 2026 14:53
Co-authored-by: Adam Petro <adam.petro@shopify.com>
Co-authored-by: Adam Petro <adam.petro@shopify.com>
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.

3 participants