Add types to ErrorFactory message parameters

[mmermerkaya]

This makes it possible to have static type checking for error parameters.

Example:

const enum ErrorCode {
  WITH_PARAMS = 'with-params',
  WITHOUT_PARAMS = 'without-params'
}

const ERROR_MAP: ErrorMap<ErrorCode> = {
  [ErrorCode.WITH_PARAMS]:
    'This message has {$param} and maybe also {$optParam}.',
  [ErrorCode.WITHOUT_PARAMS]: "This message doesn't have any parameters."
};

interface ErrorParams {
  [ErrorCode.WITH_PARAMS]: { param: string; optParam?: string };
}

const ERROR_FACTORY = new ErrorFactory<ErrorCode, ErrorParams>(
  'service',
  'serviceName',
  ERROR_MAP
);

ERROR_FACTORY.create(ErrorCode.WITH_PARAMS); // error
ERROR_FACTORY.create(ErrorCode.WITH_PARAMS, undefined); // error with strict
ERROR_FACTORY.create(ErrorCode.WITH_PARAMS, null); // error with strict
ERROR_FACTORY.create(ErrorCode.WITH_PARAMS, {}); // error
ERROR_FACTORY.create(ErrorCode.WITH_PARAMS, { param: 'param' }); // good
ERROR_FACTORY.create(ErrorCode.WITH_PARAMS, {
  param: 'param',
  optParam: 'optParam'
}); // good
ERROR_FACTORY.create(ErrorCode.WITH_PARAMS, {
  param: 'param',
  otherParam: 'otherParam'
}); // error
ERROR_FACTORY.create(ErrorCode.WITHOUT_PARAMS); // good
ERROR_FACTORY.create(ErrorCode.WITHOUT_PARAMS, undefined); // error
ERROR_FACTORY.create(ErrorCode.WITHOUT_PARAMS, null); // error
ERROR_FACTORY.create(ErrorCode.WITHOUT_PARAMS, {}); // error
ERROR_FACTORY.create(ErrorCode.WITHOUT_PARAMS, { param: 'param' }); // error
ERROR_FACTORY.create('randomCode'); // error

// This works the same for all ErrorCode values.
const ERROR_FACTORY_WITHOUT_PARAMS = new ErrorFactory<ErrorCode>(
  'service',
  'serviceName',
  ERROR_MAP
);

// Same as WITHOUT_PARAMS before, only works with a single argument.
ERROR_FACTORY_WITHOUT_PARAMS.create(ErrorCode.WITHOUT_PARAMS); // good
ERROR_FACTORY_WITHOUT_PARAMS.create(ErrorCode.WITHOUT_PARAMS, undefined); // error
ERROR_FACTORY_WITHOUT_PARAMS.create(ErrorCode.WITHOUT_PARAMS, null); // error
ERROR_FACTORY_WITHOUT_PARAMS.create(ErrorCode.WITHOUT_PARAMS, {}); // error
ERROR_FACTORY_WITHOUT_PARAMS.create(ErrorCode.WITHOUT_PARAMS, {
  param: 'param'
}); // error
ERROR_FACTORY_WITHOUT_PARAMS.create('randomCode'); // error

[Feiyang1]

You can use rest parameter with tuple type to conditionally require the second parameter. But it still doesn't work in this case, because ErrorParams[K] evaluates to unknown instead of undefined. I tried ErrorParams[K] extends unknown ? [] : [ErrorParams[K]], but it doesn't work because everything extends unknown. : (

export class ErrorFactory<
  ErrorCode extends string,
  ErrorParams extends Partial<{ readonly [K in ErrorCode]: ErrorData }> = {}
> {
  create<K extends ErrorCode>(
    code: K,
    ...data: ErrorParams[K] extends undefined ? [] : [ErrorParams[K]]
  ): FirebaseError {
  }
}

To make ErrorParams[k] extends undefined work, I changed ErrorParams interface to define fields without parameters as undefined.

interface AppErrorParams {
    [AppErrorCode.SOME_ERROR]: undefined,

    [AppErrorCode.WITH_PARAMS]: { param: string; optParam?: string };
    // See https://github.com/Microsoft/TypeScript/issues/8032
    [AppErrorCode.WITHOUT_PARAMS]: undefined;
}

Then it works as expected in all cases.
The result for your examples

ERROR_FACTORY.create(ErrorCode.SOME_ERROR); // good
ERROR_FACTORY.create(ErrorCode.SOME_ERROR, {}); // error
ERROR_FACTORY.create(ErrorCode.SOME_ERROR, { param: 'param' }); // error

ERROR_FACTORY.create(ErrorCode.WITH_PARAMS); // error
ERROR_FACTORY.create(ErrorCode.WITH_PARAMS, { param: 'param' }); // good
ERROR_FACTORY.create(ErrorCode.WITH_PARAMS, {
  param: 'param',
  optParam: 'optParam'
}); // good

ERROR_FACTORY.create(ErrorCode.WITHOUT_PARAMS); // good
ERROR_FACTORY.create(ErrorCode.WITHOUT_PARAMS, {}); // error


ERROR_FACTORY.create(ErrorCode.WITH_PARAMS, {}); // error

ERROR_FACTORY.create(ErrorCode.WITH_PARAMS, {
  param: 'param',
  otherParam: 'otherParam'
}); // error

// This one doesn't have any params.
ERROR_FACTORY.create(ErrorCode.WITHOUT_PARAMS, { param: 'whatever' });

The solution is not ideal as we need to define fields without parameters as undefined, but it's 100% type safe. Let me know if you find better solutions...

I guess {} could be allowed as the second parameter if no parameter is required, but this solution just disallows it and I think it's correct as well.

[mmermerkaya]

Thanks for taking a look!

Yeah, my first version of this was using a rest parameter as well, but it looks pretty bad in VSCode IntelliSense: create(code: ErrorCode.FILE_NOT_FOUND, __1_0: { file: string; }): FirebaseError. Second parameter appears as __0_1 if you do ...[data] and data_0 if you do ...data, so I tried to find another solution. You're right though, there's no better solution AFAICT.

For the unknown stuff, you can do this: ErrorParams[K] extends ErrorData ? [ErrorParams[K]] : [] instead of ErrorParams[K] extends undefined ? [] : [ErrorParams[K]] and it works, without explicitly defining the errors without parameters. I think it's because TS doesn't "track negated constraints in the false branch of a conditional type". So when you do ErrorParams[K] extends undefined ? [] : [ErrorParams[K]] and ErrorParams[K] isn't undefined, TS doesn't infer that the type in the false side ([ErrorParams[K]) can't be undefined, and just gives it the type unknown. They should be able to fix this issue after finishing negated types I think.

In the end, I changed it to ...data: K extends keyof ErrorParams ? [ErrorParams[K]] : []. After all that's what we really want to know, if the key is in the interface.

Changing to the rest parameter also makes this stricter, meaning if there's no entry in the interface for that ErrorCode, you can't define data for it. I tried to not make this a breaking change, but I suppose it's okay now that we're about to release a new major version. I'm happier with the result.

One last question is whether we should allow omitting ErrorParams or require an explicit {}? I'm okay with allowing it to be omitted but maybe it'll be more confusing this way, WDYT?

I updated the example in my original post.

[Feiyang1]

LGTM