Typing PayPal Card Fields onApprove per flow instead of making orderID optional

A paypal-js pull request made orderID optional to support vault flows, a breaking type change for every order integration. Typing onApprove per branch of an existing union fixed it, and the author adopted the suggestion.

Category
Code review
Published
Reading time
3 min read

The problem

PayPal Card Fields support two flows. In the order flow the integration passes createOrder, and onApprove receives an orderID. In the vault-without-purchase flow it passes createVaultSetupToken, and onApprove receives a vaultSetupToken instead. The type definitions only described the first case:

packages/paypal-js/types/components/card-fields.d.ts
export type CardFieldsOnApproveData = {
  orderID: string;
  liabilityShift?: string;
};

Issue #652 asked for vaultSetupToken and for the known liabilityShift values, which users were overriding by hand.

The proposed fix

Diff
 export type CardFieldsOnApproveData = {
-  orderID: string;
-  liabilityShift?: string;
+  orderID?: string;
+  liabilityShift?: "POSSIBLE" | "YES" | "NO" | "UNKNOWN" | string;
+  vaultSetupToken?: string | null;
 };

It adds the missing fields, but it changes two things for everyone.

What I found in review

Optional orderID breaks every order integration

With strictNullChecks, orderID?: string is string | undefined. Code that has always compiled stops compiling, even though orderID is always present in the order flow:

TypeScript
captureOrder(data.orderID) // where captureOrder(id: string)
// error: Argument of type 'string | undefined' is not assignable to parameter of type 'string'

That is a breaking change shipped in a .d.ts file. The types already had what was needed: PayPalCardFieldsComponentOptions is a union of PayPalCardFieldsComponentCreateOrder and PayPalCardFieldsComponentCreateVaultSetupToken, where each branch sets the other's function to never. TypeScript picks the branch from the key that is present, and a contextually typed callback gets that branch's parameter type. So onApprove can be typed per branch:

TypeScript
export type CardFieldsOnApproveData = {
  orderID: string;
  liabilityShift?: CardFieldsLiabilityShift;
};
export type CardFieldsVaultOnApproveData = {
  vaultSetupToken: string;
  liabilityShift?: CardFieldsLiabilityShift;
};

Moving onApprove from the shared base interface into the two branches keeps orderID required where it always exists and gives vault integrations a required, typed token.

A literal union that collapses to string

"POSSIBLE" | "YES" | "NO" | "UNKNOWN" | string simplifies to string, because every literal is already a string. The literals disappear: no narrowing and no editor completions. The usual way to suggest known values while accepting new ones is to stop the union from collapsing:

TypeScript
export type CardFieldsLiabilityShift = "POSSIBLE" | "YES" | "NO" | "UNKNOWN" | (string & {});

string & {} still accepts any string, but TypeScript does not merge the literals into it, so they stay visible.

Tests that test the type

The new test assigned values and read them back, which passes with the old type too. For type definitions the test has to fail at type-check time: expectTypeOf assertions, and @ts-expect-error on the shape that must not compile, such as reading vaultSetupToken in a createOrder configuration. I also noted that the repository asks for a changeset.

What happened next

The author replied the next morning and pushed commit 0694868. The diff matches each point:

TypeScript
onApprove: (data) => {
  expectTypeOf(data).toEqualTypeOf<CardFieldsOnApproveData>();
  expectTypeOf(data.orderID).toBeString();
  // @ts-expect-error: vaultSetupToken does not exist on CardFieldsOnApproveData in createOrder config
  const token = data.vaultSetupToken;
},

Takeaways

  • In a type definition package, making a field optional is a breaking change. Look for an existing union before widening a shared type.
  • "a" | "b" | string is just string. Use (string & {}) when you want suggestions without a closed set.
  • A runtime test cannot test a type. expectTypeOf and @ts-expect-error can.

References

Code and history behind this post

More

Theme