feat: add easly accessable handler in order to make unit tests easier

[czlowiek488]

Closes #

Checklist:

• [x] I have updated the documentation accordingly.
• [ ] I have read the CONTRIBUTING document.

Previously in order to unit test handler I had to put it outside of composeHandler and types were gone. In order to make handler unit testable with fully working type system I assigned a variable to the function itself (not a common scenario) at the end of composeHandler.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[dbartholomae]

Thanks! Unfortunately, I don't think that this is a pattern that I think this library should encourage, as it encourages to write handlers with complex obscure parameter and return types, instead of having clean and easy interfaces that can be defined explicitly next to a handler function. Do you have an example where this kind of behavior would be beneficial compared to having the handler exported separately from a separate file?

[czlowiek488]

Ok, here is my use case.

export const handle = composeHandler(
  middlewareErrorHandler(),
  middlewareLogger(),
  middlewareJsonParser(),
  middlewareValidator(lambdaSchemaPackageAdd),
  middlewareLoadDependencies(["repositoryPackage", "integrationNpmJs"]),
  async (event) => {
    const existingPackage = await event.dependencies.repositoryPackage.findPackage({
      packageName: event.jsonBody.packageName,
    });
    if (existingPackage !== null) {
      throw new ConflictError("package must not already exists in database");
    }
    const packageInfo = await event.dependencies.integrationNpmJs.findPackageInfo({
      packageName: event.jsonBody.packageName,
    });
    if (packageInfo === null) {
      throw new ConflictError("package.packageName must exists in npm registry");
    }
    await event.dependencies.repositoryPackage.createPackage({
      packageName: event.jsonBody.packageName,
      packageStatus: event.jsonBody.packageStatus,
    });
    return awsLambdaResponse(StatusCodes.OK, {}, {});
  },
);

As you can see I'm doing DI through middleware and dependencies are available in event variable. I also have zod validation library which let me infer types from the schema inside of middleware. I need to have those types infered from middlewares to achieve great developer experience.

In order to unit test my handler I need to access it directly without middlewares.

export const handler = async (event) => {  // types are gone here :(
    const existingPackage = await event.dependencies.repositoryPackage.findPackage({
      packageName: event.jsonBody.packageName,
    });
    if (existingPackage !== null) {
      throw new ConflictError("package must not already exists in database");
    }
    const packageInfo = await event.dependencies.integrationNpmJs.findPackageInfo({
      packageName: event.jsonBody.packageName,
    });
    if (packageInfo === null) {
      throw new ConflictError("package.packageName must exists in npm registry");
    }
    await event.dependencies.repositoryPackage.createPackage({
      packageName: event.jsonBody.packageName,
      packageStatus: event.jsonBody.packageStatus,
    });
    return awsLambdaResponse(StatusCodes.OK, {}, {});
  };
export const handle = composeHandler(
  middlewareErrorHandler(),
  middlewareLogger(),
  middlewareJsonParser(),
  middlewareValidator(lambdaSchemaPackageAdd),
  middlewareLoadDependencies(["repositoryPackage", "integrationNpmJs"]),
  handler,
);

In my unit test I mock all dependencies and incoming data.

I would love to see handler to be inside composeHandler function to let typescript infer types automatically from the composition chain. As far as I know what I suggested in PR is the only way to achieve both automatic type inference and unit testable handler.

If you would like to reject this PR it's fine, I will create fork just for myself.

[dbartholomae]

Thanks! I've thought a bit about this, and would recommend to just add explicit types. The compose and composeHandler functions are also very small parts with almost 0 probability of change, so that the cost of having to create a local copy if you want to add the .handler as in this PR is quite low.

[czlowiek488]

To be honest I don't really understand your point of view. Suggestions you made will do the job, but I don't really see using any of those while developing quality product. Thanks for quick response anyway.

[czlowiek488]

If someone would end up in this thread here is a package I prepared for with this change and also using ts-pipe-compose package rather than writing own implementation of composition. https://www.npmjs.com/package/middleware-composer

[dbartholomae]

Thanks! I could add a link in a "related project" section of the README if you want :)