Skip to content

Clean quicktype model names#224

Open
kiftio wants to merge 10 commits into
mainfrom
clean-quicktype-model-names
Open

Clean quicktype model names#224
kiftio wants to merge 10 commits into
mainfrom
clean-quicktype-model-names

Conversation

@kiftio
Copy link
Copy Markdown
Contributor

@kiftio kiftio commented May 27, 2026

What changes are you making?

  • Generate protocol models from root schemas instead of every types/*.json schema.
  • Keep types/error_response.json as an explicit root because CheckoutProtocol.error exposes ErrorResponse.
  • Fix extracted OpenRPC result schemas to update the oneOf[0] success branch and reuse canonical checkout payment.
  • Normalize remaining exact quicktype fallback names.

Diffs

Removed Class suffix duplicates

- BuyerClass          → Buyer (kept)
- ContextClass        → Context (kept)  
- PaymentClass        → Payment (kept)
- ItemClass           → Item (kept)
- CredentialClass     → CredentialObject removed (only PaymentCredential kept)
- FulfillmentClass    → Fulfillment (kept)
- SignalsClass        → Signals (kept)

Removed Element suffix duplicates

- AdjustmentElement   → Adjustment (kept)
- MessageElement      → Message (kept)
- LinkElement         → Link (kept)
- LineItemElement     → (merged into LineItem/OrderLineItem)
- ExpectationElement  → Expectation (kept)
- EventElement        → (merged into FulfillmentEvent)

Removed quicktype color fallbacks

- PurpleSelectedPaymentInstrument → SelectedPaymentInstrument
- PurpleService                   → InstrumentsChangeService
- PurpleStatus                    → StatusEnum

Consolidations

- CheckoutLineItem + LineItemElement → LineItem
- OrderLineItemQuantity + LineItemQuantity → Quantity
- FulfillmentEventLineItem duplicates merged
- Various "Total" classes consolidated
Example
Before

1. CheckoutLineItem (used in Checkout)

class CheckoutLineItem(
    val id: String,
    val item: ItemClass,
    val parentId: String?,
    val quantity: Long,              // simple number
    val totals: List<LineItemTotal>
)

2. LineItem (duplicate of CheckoutLineItem)

class LineItem(
    val id: String,
    val item: ItemObject,             // different suffix
    val parentId: String?,
    val quantity: Long,               // simple number
    val totals: List<LineItemTotal>
)

3. LineItemElement (used in Order)

class LineItemElement(
    val id: String,
    val item: ItemClass,
    val parentId: String?,
    val quantity: LineItemQuantity,   // complex object with fulfilled/original/total
    val status: OrderLineItemStatus,
    val totals: List<LineItemTotal>
)

After

1. LineItem (for checkout, simple quantity)

class LineItem(
    val id: String,
    val item: Item,                   // clean name
    val parentId: String?,
    val quantity: Long,               // simple
    val totals: List<LineItemTotal>
)

2. OrderLineItem (for orders, complex quantity)

class OrderLineItem(
    val id: String,
    val item: Item,
    val parentId: String?,
    val quantity: Quantity,           // complex with fulfilled/original/total
    val status: LineItemStatus,
    val totals: List<LineItemTotal>
)

So the consolidation:

  • Merged CheckoutLineItem + LineItem duplicates → LineItem
  • Renamed LineItemElementOrderLineItem (clearer purpose)
  • Cleaned up type names (ItemClassItem, LineItemQuantityQuantity)

Major removals

- All the merchant/business/platform config classes
- Many intermediate fulfillment types
- Duplicate payment instrument variations

Some of these were defined in types/*.json but not actually used in the current Checkout or Order schemas.

Some things were separate MessageError, MessageInfo, MessageWarning, but are now consolated into Message with a type discriminator


The net effect: ~70% fewer generated types, all with cleaner names, while the public API surface (Checkout, Order, ErrorResponse, results) remains stable.

Why

Passing every reusable schema as --src made quicktype emit duplicate/fallback models such as BuyerClass, PaymentObject, AdjustmentElement, and PurpleSelectedPaymentInstrument.

The new generation keeps callback payload roots intact while using cleaner nested model names.

Review guide

Primary review files:

  • protocol/scripts/generate_models.sh
  • platforms/android/lib/api/lib.api

Generated files are large because quicktype reordered/collapsed duplicate model declarations.

API impact

Breaking generated-model API cleanup during alpha.

Public callback roots remain:

  • Checkout
  • ErrorResponse
  • WindowOpenRequest
  • WindowOpenResult

Nested callback model names changed, for example:

  • BuyerClass -> Buyer
  • ContextClass -> Context
  • PaymentClass -> Payment
  • PaymentSelectedPaymentInstrument -> SelectedPaymentInstrument
  • MessageElement -> Message
  • LinkElement -> Link

Some standalone generated types are no longer emitted because they were only generated from broad types/*.json roots and are not reachable from current generated roots.

Before you merge

Important

  • I've added tests to support my implementation
  • I have read and agree with the Contribution Guidelines
  • I have read and agree with the Code of Conduct
  • I've updated the relevant platform README (platforms/swift/README.md and/or platforms/android/README.md)

Releasing a new Swift version?
  • I have bumped the version in ShopifyCheckoutKit.podspec
  • I have bumped the version in platforms/swift/Sources/ShopifyCheckoutKit/ShopifyCheckoutKit.swift
  • I have updated platforms/swift/CHANGELOG.md
  • I have updated the SwiftPM/CocoaPods version snippets in platforms/swift/README.md (major version only)
Releasing a new Android version?
  • I have bumped the versionName in platforms/android/lib/build.gradle
  • I have updated platforms/android/CHANGELOG.md
  • I have updated the Gradle/Maven version snippets in platforms/android/README.md

Tip

See the Contributing documentation for the full release process per platform.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 27, 2026

React Native — Coverage Report

Lines Statements Branches Functions
Coverage: 92%
91.59% (316/345) 87.25% (178/204) 100% (82/82)

@kiftio kiftio marked this pull request as ready for review May 28, 2026 08:34
@kiftio kiftio requested a review from a team as a code owner May 28, 2026 08:34
Copy link
Copy Markdown
Contributor

kieran-osgood-shopify commented May 28, 2026

I'm worried we're starting to go too deep on the sed usage which makes codegen brittle, it works for this version of the spec, but depending on quicktype formatting, language keywords, future naming object names makes me nervous the more we build upon it

I think it was more fine for changing keywords for the language where they are more stable such as the

 sed -i '' '1,/^package /{/^package /!d;}' "${OUTPUT}"
   sed -i '' \
     -e 's/^data class /public data class /' \
     -e 's/^sealed class /public sealed class /' \
     -e 's/^enum class /public enum class /' \
     -e 's/^typealias /public typealias /' \
     -e 's/^    class /    public class /' \
     -e 's/^    val /    public val /' \
     -e 's/(val value: /(public val value: /' \
     "${OUTPUT}"

The fact quicktype falls back to all this "Purple" names, we run into conflicts with platform (e.g. Binding / ColorScheme) is a smell of bad input though


I'm wondering whether we could explore a pre-quicktype-processing stage in the generation.
QuickType will first look for a title property in nodes it generates for to get the symbol name, if we add a jq step to add the 'parent' node name in to end nodes it'll provide unambiguous names more deterministically, we focus on manipulating the UCP source we copy into the repo centrally, then generate the 'codegen' separately

To give a concrete example, there are 3 instances of Type under 3 different message related objects.
Prefixing them with Message or uniquely with ErrorMessage / WarningMessage etc. would remove the awkward name its assigned (we can make the call of whether to merge similar classes like this together, whilst merging is good for bundle size, any future deviation on these typese in future will be a breaking change for consumers.)
image.png


Few other notes aside:

  • BillingAddressClass goes away and is now merged with PostalAddress - this seems fine right?
  • These types are flat out removed - but seems intentional because we dont do payment delegation? Just checking here!
  BusinessFulfillmentConfig
  MerchantFulfillmentConfig
  PlatformFulfillmentConfig
  CardCredential
  TokenCredential
  CardPaymentInstrument
  PaymentIdentity
  PaymentAccountInfo
  FulfillmentAvailableMethod
  FulfillmentDestination
  FulfillmentGroup
  FulfillmentMethod
  FulfillmentOption
  RetailLocation
  ShippingDestination

@kiftio
Copy link
Copy Markdown
Contributor Author

kiftio commented May 28, 2026

@kieran-osgood-shopify thanks for the review, missed this initially..

I think the latest approach addresses the sed concern, we're just down to a couple of Purple prefix seds


R.e. the removed classes, it looks like we're no longer including extensions, just the base schemas. Given current kit enabled delegations, that might be correct - worth checking into though.

BillingAddressClass merging into PostalAddress seems mostly ok. billing_address points at postal_address.json. I suppose there's some risk of future divergence between the two

@kiftio
Copy link
Copy Markdown
Contributor Author

kiftio commented May 28, 2026

@kieran-osgood-shopify thanks for the review, missed this initially..

I think the latest approach addresses the sed concern, we're just down to a couple of Purple prefix seds

R.e. the removed classes, it looks like we're no longer including extensions, just the base schemas. Given current kit enabled delegations, that might be correct - worth checking into though.

BillingAddressClass merging into PostalAddress seems mostly ok. billing_address points at postal_address.json. I suppose there's some risk of future divergence between the two

I've now restored some of the extension files

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