Incorrect types for optional array params

Making Packs
1

My Zapier Hooks Pack recently broke for a few hours for something that TypeScript should have caught, but didn’t due to incorrect types or due to incorrect initialisation.

Here’s the formula coda:

pack.addFormula({
	name: 'ZapierHook',
	description: 'Trigger a Zapier Hook',
	parameters: [
		coda.makeParameter({
			type: coda.ParameterType.String,
			name: 'url',
			description: 'The Webhook URL of the Zapier Zap',
		}),
		coda.makeParameter({
			type: coda.ParameterType.String,
			name: 'body',
			description: 'Body for a Zapier Catch Raw Hook',
			optional: true,
		}),
		coda.makeParameter({
			type: coda.ParameterType.StringArray,
			name: "bodyParams",
			description: "Body Params in the form of List(key, value, ...) for a Zapier Catch Hook",
			optional: true,
		}),
		coda.makeParameter({
			type: coda.ParameterType.StringArray,
			name: "queryParams",
			description: "Query/Search Params in the form of List(key, value, ...) for a Zapier Catch Hook",
			optional: true,
		}),
	],
	resultType: coda.ValueType.String,
	isAction: true,
	execute: async function ([where, body, bodyParamsList = [], queryParamsList = []], context) {
		// ...
	}
})

The issue is that optional StringArray params, such as:

		coda.makeParameter({
			type: coda.ParameterType.StringArray,
			name: "bodyParams",
			description: "Body Params in the form of List(key, value, ...) for a Zapier Catch Hook",
			optional: true,
		}),

Have their type set to string[] instead of string[] | undefined:

CleanShot 2022-09-16 at 11.58.24
CleanShot 2022-09-16 at 11.58.241574×164 50.5 KB

This then leads the pack author to assume that the param is pre-initialised by coda to an empty array (it isn’t) which can cause the pack to break as typescript won’t catch errors due to this mismatch (such as doing a .length check on an undefined value).

Instead, what the pack author must do is initialise the param to an empty array:

execute: async function ([where, body, bodyParamsList = [], queryParamsList = []], context) {

So request here is for either the correct type to be returned, or for the variable to be initialised to the correct type.

Relevant pack code on github:

2 Likes
2

I’m really out of my depth here, but isn’t SparseArray designed to overcome this challenge?

3

Thanks for the report @balupton. It looks like the issue here is that our typing logic doesn’t take into account the optional field of the parameter definition when calculating the type of the arguments to the execute function. I think the relevant code is here:

I’m honestly a bit out of my depth when it comes to advanced TypeScript typings, but would you be up for submitting a PR? If not I can put this on the queue for the engineering team, but it may be lower priority.

1 Like
4

Seems typescript can’t do that advanced inferance yet, and that it needs the actual code (not just the types) to send an is

For things that should have worked:

export interface BaseParamDef<T extends UnionType> {
    /**
     * The name of the parameter, which will be shown to the user when invoking this formula.
     */
    name: string;
    /**
     * The data type of this parameter (string, number, etc).
     */
    type: T;
    /**
     * A brief description of what this parameter is used for, shown to the user when invoking the formula.
     */
    description: string;
    /**
     * Whether this parameter can be omitted when invoking the formula.
     * All optional parameters must come after all non-optional parameters.
     */
    // optional: boolean;
    /**
     * A {@link MetadataFormula} that returns valid values for this parameter, optionally matching a search
     * query. This can be useful both if there are a fixed number of valid values for the parameter,
     * or if the valid values from the parameter can be looked up from some API.
     * Use {@link makeMetadataFormula} to wrap a function that implements your autocomplete logic.
     * Typically once you have fetched the list of matching values, you'll use
     * {@link autocompleteSearchObjects} to handle searching over those values.
     * If you have a hardcoded list of valid values, you would only need to use
     * {@link makeSimpleAutocompleteMetadataFormula}.
     */
    autocomplete?: MetadataFormula;
    /**
     * @deprecated This will be removed in a future version of the SDK. Use {@link ParamDef.suggestedValue} instead.
     */
    defaultValue?: SuggestedValueType<T>;
    /**
     * The suggested value to be prepopulated for this parameter if it is not specified by the user.
     */
    suggestedValue?: SuggestedValueType<T>;
}
interface RequiredParamDef<T extends UnionType> extends BaseParamDef<T> {
    optional?: false;
}
interface OptionalParamDef<T extends UnionType> extends BaseParamDef<T> {
    optional: true;
}
export type ParamDef<T extends UnionType> = OptionalParamDef<T> | RequiredParamDef<T>;

/** @hidden */
export declare type ParamArgs<T extends UnionType> = Omit<ParamDef<T>, 'description' | 'name' | 'type'>;
/**
 * The type for the complete set of parameter definitions for a fomrula.
 */
export declare type ParamDefs = [ParamDef<UnionType>, ...Array<ParamDef<UnionType>>] | [];
/** @hidden */
export declare type ParamsList = Array<ParamDef<UnionType>>;
declare type TypeOfMap<T extends UnionType> = T extends Type ? TypeMap[T] : T extends ArrayType<infer V> ? T extends SparseArrayType<infer V> ? Array<TypeMap[V] | undefined> : Array<TypeMap[V]> : never;
/**
 * The type for the set of argument values that are passed to formula's `execute` function, based on
 * the parameter defintion for that formula.
 */
export declare type ParamValues<ParamDefsT extends ParamDefs> = {
    [K in keyof ParamDefsT]:
		ParamDefsT[K] extends OptionalParamDef<infer T> ? TypeOfMap<T>? :
		ParamDefsT[K] extends RequiredParamDef<infer T> ? TypeOfMap<T> : never;
} & any[];

Or even most simply:

export declare type ParamValues<ParamDefsT extends ParamDefs> = {
    [K in keyof ParamDefsT]:
		ParamDefsT[K] extends ParamDef<infer T>
		? ParamDefsT[K]['optional'] extends true ? TypeOfMap<T>? : TypeOfMap<T>
		: never;
} & any[];

That said, filtering by type does work.

Here’s the stream where I try a whole bunch of things.

1 Like
5

Until something more precise is figured out, I think in the meantime it’s better to be safe than sorry. This will make all params possibly null, which will force people to set a default value, like I have done in the OP, or to check and error:

export declare type ParamValues<ParamDefsT extends ParamDefs> = {
    [K in keyof ParamDefsT]: ParamDefsT[K] extends ParamDef<infer T> ? TypeOfMap<T>? : never;
} & any[];

It’s much easier to change:

	execute: async function (
		[where, body, bodyParamsList, queryParamsList],
		context
	) {

To:

	execute: async function (
		[where = '', body = '', bodyParamsList = [], queryParamsList = []],
		context
	) {

Than to respond to support requests with late night uncaught exceptions.

Opened a PR with this suggestion: params can be undefined by balupton · Pull Request #2210 · coda/packs-sdk · GitHub

The alternative, as per the OP, is to have the caller of execute initialise all param values, even those that are optional, to empty values of the correct types: however for object types, that would be confusing, and imho not an accurate trade-off compared to the above suggestion.

1 Like
6

Figured out why optional was getting discarded; the code that generates parameters results in a type that only persists the parameter type:

The typing there just says the function returns a paramdef that matches the parameter type.

Also updating the .d.ts seems to be the wrong place for changes, as it seems it is compiled from:

Which also requires changes for this support.

1 Like
7

Thanks for doing a ton of research here, and sorry for the late reply. As mentioned, advanced TypeScript typing is out of my depth, so I’m not sure I have much to add here. It looks like you and @JonathanGoldman are having some dialog in the PR, so it probably makes sense to continue the dialog there.

1 Like
8

Yeah, I figured out how to do it properly in theory and it works on a simplified representation of the code in question here, but given the complexities of the SDK I think I’m running into a TypeScript inference bug, still hunting for a good solution here.

2 Likes
9

@balupton We just released version 1.1.0 which has the proper typing for optional parameters now. If you’re using the CLI you can install it at your convenience, if you’re using Pack Studio it should be in effect there by the end of the week.

2 Likes