chore(parser): fix type inference for result types

[am29d]

Summary

This PR fixes inferred for result types.

Changes

I have added discriminator field to each envelope to distinguish between array and object return types. We now also have tests to catch any cases where the return type for specific envelope does not match.

Technically the object-envelopes can also return arrays, but this result will be according to the provided schema, i.e. z.array which also matches with current implementation.

Please provide a summary of what's being changed

Please add the issue number below, if no issue is present the PR might get blocked and not be reviewed

Issue number: closes #3226

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

[dreamorosi]

Thank you for the fix, this will improve the experience for customers.

Besides two minor comments on imports within tests, I have noticed that the new symbol property shows up in the IntelliSense autocomplete of the public API:

This is not a big deal, but I wonder if we could use alternatives to avoid this. Based on tweets that I read in the past there are at least two options that would avoid this and still serve as discriminator.

The first one is to use a property name that begins with a space, i.e. this:

export const ApiGatewayEnvelope = {
-  'symbol': 'object' as const,
+  ' symbol': 'object' as const,

This works, however according to some sources the fact that it doesn't show might actually be a bug.

The second is to use an actual symbol, for example, in packages/parser/src/envelopes/envelope.ts we could export a symbol like this:

export const envelopeDiscriminator = Symbol.for('returnType');

and then, in each built-in envelope import and use it like this:

- import { Envelope } from './envelope.js';
+ import { Envelope, envelopeDiscriminator } from './envelope.js';

/**
 * API Gateway envelope to extract data within body key
 */
export const ApiGatewayEnvelope = {
-  'symbol': 'object' as const,
+  [envelopeDiscriminator]: 'object' as const,

and same in the packages/parser/src/types/envelope.ts file:

+ import { envelopeDiscriminator } from '../envelopes/envelope.js';

interface ArrayEnvelope {
-  'symbol': 'array';
+  [envelopeDiscriminator]: 'array';
  parse<T extends ZodSchema>(data: unknown, schema: T): z.infer<T>[];
  safeParse<T extends ZodSchema>(
    data: unknown,
    schema: T
  ): ParsedResult<unknown, z.infer<T>[]>;
}

with this method the types still work but the discriminator doesn't show up in the public API:

Using a symbol vs a string as key would also, afaik, avoid potential collisions in case customers start adding properties to the envelopes since even if they used the same key the reference would be different (but haven't verified this).

Additionally, I think we could consider & test adding the @internal annotation in the docstring for the object property so that it doesn't show up in the API docs either.

Finally, just as a comment, I think we might have been able to achieve the same result by adding the discriminator only to one group of the envelopes (i.e. the array ones) and then setting the type of the other as never in the interface, so that the absence of the discriminator would have served as discriminator itself. But I actually like being explicit about this, so I'd say let's leave it as-is.

[am29d]

Thanks for raising this point, I completely forgot about hiding the property.

I have picked your second option, and added @hidden to remove it from our API Docs. I agree on the last point, let's make it explicit.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[dreamorosi]

Thank you for addressing the review comments! Great addition