TypeScript improvements - what should we address?

[lmammino]

Hello all, we have received interest from an organization willing to support (financially) a rework of TypeScript support in Middy.
What we are currently thinking is to use this opportunity to try our best to improve the DX of using Middy with TypeScript.

This is the current high-level scope that we have in mind:

• Update to leverage new features in TS v5
• Refactor core types and how middleware types work Distinguish between before and after in middleware type signatures #914
• Refactor middlewares to reflect core changes and be consistent
• Improve TS tests
• Add in more documentation on how to use middy with TS, add examples and write custom middleware using TS
• Switch back to TS JSDoc? TS to JSDoc Conversion sveltejs/svelte#8569 / https://news.ycombinator.com/item?id=35892250 / https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html
• Middy does not export types correctly #1071

@community: please link any past closed issues that you feel should be revisited as part of this initiative. Also if you have any other comments or feedback, please do share your view.

If you just want to leave us a reaction, please go with these:

• 👍 This would be helpful to me and my team
• 🎉 I'm able to help with scoping
• 👀 I'm able to code review when a PR is ready
• 🚀 I'm able to test before it's released

UPDATE

This conversation has lead to the definition of some issues that will be worked on by the community and the maintainers:

Based on what has been discussed so far in this thread, I believe this is an actionable list of tasks that we could realistically try to tackle (hopefully in time for v5):

• TypeScript improvement: getInternal type safety #1084 (suggested by @m-radzikowski)
• TypeScript improvement: @middy/http-json-body-parser should support API Gateway V2 events #1085 (suggested by @apalchys)
• TypeScript improvement: Improve documentation to highlight middleware-first, handler-last usage #1086 (suggested by @bilalq)
• TypeScript improvement: Replace any with unknown where it makes sense #1087 (suggested by @bilalq)
• TypeScript improvement: Context should keep track of changes made by middleware(s) #1088 (suggested by @ben-eb)

Please let us know in this thread if you are interested in contributing to some of these issues

[KillDozerX2]

I'm happy to allocate some of our devs(I have 2 in mind) that can work on implementation. Apart from that, our team is available in helping you guys with scoping and we can share what we're doing with our current TS setup.

[naorpeled]

Would love to help with the implementation of this ❤️

[lmammino]

Hello @KillDozerX2 and @naorpeled

Apologies for the delayed reply and thanks a lot for offering help.

I'd be curious to know what pain points you are seeing with the current TS setup and what you have in mind to improve things.

Please, bear in mind that the current team of maintainers doesn't have a hell of a lot of TS experience, so we hope to rely as much as possible on the guidance of the community.

Once we are able to define the scope and come up with a plan we can start to see who can help with what and discuss potential financial rewards with out potential sponsor and see if we can put this machine in motion.

[m-radzikowski]

My two cents:

Middy data fetching middlewares are hard to use with TypeScript.

If you set values to context, those properties do not exist in @types/aws-lambda Context.

If you use getInternal, there is no type safety either:

let params: {apiKey: string};

export const handler = middy(lambdaHandler)
	.use(
		ssm({
			fetchData: {
				apiKey: 'my-api-key-param',
			},
		}),
	)
	.before(async request => {
		params = await getInternal(["apiKey"], request);
	});

The getInternal function:

• has any type for variables
• returns Promise<any>

I think both problems could potentially be solved:

• if we setToContext: true then the request type is modified to include apiKey, we could use similar mechanism here to infer what's in the request props accessed by getInternal()?
• then the return type can be inferred from the array params, I think.

[apalchys]

List of issues my team see (I'm going to keep it up to date):

• @middy/http-json-body-parser defines event using legacy APIGatewayEvent which describes event payload format v1. No Payload format v2 support.

[willfarrell]

@apalchys If you'd like to open a PR to add v2 format support it can include it in the current version.

Thanks for sharing

[RichiCoder1]

Honestly, I think one of the biggest quirks is it's hard to flow middleware types into functions because Middy goes function -> middleware -> middleware instead of middleware -> middleware -> function like other request frameworks when defining the handler.

[lmammino]

My two cents:

Middy data fetching middlewares are hard to use with TypeScript.

If you set values to context, those properties do not exist in @types/aws-lambda Context.

If you use getInternal, there is no type safety either:

let params: {apiKey: string};

export const handler = middy(lambdaHandler)
	.use(
		ssm({
			fetchData: {
				apiKey: 'my-api-key-param',
			},
		}),
	)
	.before(async request => {
		params = await getInternal(["apiKey"], request);
	});

The getInternal function:

• has any type for variables
• returns Promise<any>

I think both problems could potentially be solved:

• if we setToContext: true then the request type is modified to include apiKey, we could use similar mechanism here to infer what's in the request props accessed by getInternal()?
• then the return type can be inferred from the array params, I think.

Thanks, @m-radzikowski. This is indeed a valid point!

I am not too sure I am following your suggestion, though?

When you say:

• if we setToContext: true then the request type is modified to include apiKey, we could use similar mechanism here to infer what's in the request props accessed by getInternal()?
• then the return type can be inferred from the array params, I think.

This is probably due to my own TS ignorance, but could you provide an example (even pseudo code) on how you would envision this to work?

[lmammino]

Honestly, I think one of the biggest quirks is it's hard to flow middleware types into functions because Middy goes function -> middleware -> middleware instead of middleware -> middleware -> function like other request frameworks when defining the handler.

Tnx, @RichiCoder1. Do you mean in terms of how the middlewares are actually executed or how they are declared? Would changing something make it easier to support TS?

I think we should be open to considering options that might result in breaking changes if we feel they would considerably improve the DX...

[bilalq]

Honestly, I think one of the biggest quirks is it's hard to flow middleware types into functions because Middy goes function -> middleware -> middleware instead of middleware -> middleware -> function like other request frameworks when defining the handler.

Middy already supports going in either order. We use Middy 3.x with TypeScript today without really hitting any problems. If you have middlewares that mutate the request or context, so long as the middleware is typed correctly, your end handler gets the correct types. You start out with your base handler typing, and then the middlewares transform the types as they run until your .handler call.

Example:

import type { APIGatewayProxyEvent, APIGatewayProxyResult } from 'aws-lambda'
import middy from '@middy/core'
import secretsManager from '@middy/secrets-manager'

export const handler = middy<APIGatewayProxyEvent, APIGatewayProxyResult>()
  .use(
    secretsManager({
      fetchData: {
        apiToken: 'dev/api_token'
      },
      awsClientOptions: {
        region: 'us-east-1'
      },
      setToContext: true
    })
  )
  .handler(async (req, context) => {
    // The context type gets augmented here by the secretsManager middleware.
    // This is just an example, obviously don't ever log your secret in real life
    console.log(context.apiToken)
    return {
      statusCode: 200,
      body: JSON.stringify({
        message: 'OK',
        req
      }),
    }
  })

I think what's missing is just documentation and examples here.

[bilalq]

As @m-radzikowski mentioned, the actual middleware typings could be improved. It'd be a breaking change, but types should use unknown instead of any. That would make them safe and force clients to validate their types match expectations without letting them unsafely access properties.

In the case of the secretsManager middleware, it would be even better for it to take an optional typeguard function. That would allow for exact type awareness without needing to resort to unknown or any. This would be useful even for non-TS consumers that are just using vanilla JS. If a secret value is fetched and it's shape doesn't match expectations, an error gets thrown, so the rest of the code can safely assume the secret shape is as expected.

As a general observation, I don't think there are issues with the general typing of middy itself, but there are improvements that could be made to the official middlewares.

Edit: Actually, this point about before/after types is pretty valid. We haven't run into that problem ourselves, but I can certainly see why others would.

[lmammino]

@bilalq do you, or someone else reading this thread, have an appetite for helping with more docs and examples? Also happy to receive a PR changing any to unknown or other type improvements :)

[RichiCoder1]

Middy already supports going in either order. We use Middy 3.x with TypeScript today without really hitting any problems. If you have middlewares that mutate the request or context, so long as the middleware is typed correctly, your end handler gets the correct types. You start out with your base handler typing, and then the middlewares transform the types as they run until your .handler call.

Ahh, that's entirely my bad then 😅. I had recollection of trying this in the past and running into a fair bit of issues.

Docs would def help a lot here!

[baloran]

Hello! Thanks for your work ! One thing that would be interesting is to support type passing through the middleware to the handler. For example, for validator middleware. Passing types would be interesting to prevent having double validation for typescript project.

[m-radzikowski]

Sorry for no responding earlier. Better late than never?

When you say:

• if we setToContext: true then the request type is modified to include apiKey, we could use similar mechanism here to infer what's in the request props accessed by getInternal()?
• then the return type can be inferred from the array params, I think.

This is probably due to my own TS ignorance, but could you provide an example (even pseudo code) on how you would envision this to work?

If you do:

middy(handler)
    .use(
        ssm({
            fetchData: {
                sampleParam: '/mySampleParam',
            },
            setToContext: true,
        }),
    )
    .before(async request => {
        request.context.sampleParam // <--- the type is JsonValue
    })

In before(), the request.context is enriched with parameters from fetchData. This is done with this line:

middy/packages/ssm/index.d.ts

Line 17 in 9db002b

Record<ExtractSingles<keyof TOptions['fetchData']>, JsonValue> &

And it's great! It conditionally modifies output Context type from ssm() middleware, adding new keys to it.

Theoretically, the same mechanism could be applied to getInternal(). The MiddlewareObj could have another generic type with internal keys. Initially it's empty, but next middleware could add to it. Pseudo code:

export interface MiddlewareObj<
  TEvent = unknown,
  TResult = any,
  TErr = Error,
  TContext extends LambdaContext = LambdaContext,
  TInternal = {}, // <----- new generic value
> {
  before?: MiddlewareFn<TEvent, TResult, TErr, TContext>
  after?: MiddlewareFn<TEvent, TResult, TErr, TContext>
  onError?: MiddlewareFn<TEvent, TResult, TErr, TContext>
}

export type Internal<TOptions extends Options> = Record<ExtractSingles<keyof TOptions['fetchData']>, JsonValue>

declare function ssm<TOptions extends Options> (
  options?: TOptions
): middy.MiddlewareObj<unknown, any, Error, Context<TOptions>, Internal<TOptions>>

Then in the before() the request has the type with possible key values that can be used in getInternal() call.

---

However, I see it's much easier to extract values right now from the context. But the type of ssm param values is JsonValue from type-fest. I suspect this is to support both String and StringList parameters. But is it possible to get anything else than string, string[]? If parameter is not found, an exception is thrown. So I think it can be only a string or a string array.

---

I also did not know you can do handler at the end. All examples I saw have the handler at top.

It's maybe less elegant than having the handler at the top of the file, but you get Context extended with param names 👍

---

Another typing problem in middy is with event-normalizer. If you use @types/aws-lambda (and probably everyone doing TS is) you get nicely normalized event, but incompatible with types. Would it be possible to return the correct type based on the provided "raw" type from @types/aws-lambda? So if "raw" event type is KinesisStreamEvent, return similar type but with data being JsonObject (or JsonValue), etc.

[avdwio]

Hey @lmammino / @willfarrell , it's been a while since there's been much of an update here. Hypothetically, if I opened up a non-trivial PR (with potential breaking changes?), would that be welcome? Or is work underway for v5?

IMO there are good gains to be had on the TS front but I'm conscious of repeating work that might already be in progress. (eg. I think #914 is a good start)

[willfarrell]

@avdwio Thanks for your interest in helping improve TS on Middy. There is a branch release/5.0 that already has all planned changes for v5 (excluding TS changes for this issue). So any changes should be made off of that branch. I've asked @lmammino to lead/organize on this issue, I'm not sure if any changes have been started or not. I'll leave it to him to comment on that.

[ben-eb]

Hello, thank you for your work on Middy 🙂

It would definitely be nice if the internal types kept track of extensions made by the middleware(s) to the Context object. Whilst the following type checks nicely;

import {Context as LambdaContext} from "aws-lambda";
import middy from "@middy/core";
import secretsManager from "@middy/secrets-manager";

type MyContext = LambdaContext & {
    mySecret: { key: string }
};

export const handler = middy<unknown>()
    .use(secretsManager({
        fetchData: {
            mySecret: 'mySecret'
        },
        setToContext: true
    }))
    .handler(async (event: unknown, context: MyContext) => {
        // don't log your actual secrets in production :)
        console.log(context.mySecret.key);
    });

Trying to use this handler in test code will result in the following TypeScript error:

TS2345: Argument of type 'Context' is not assignable to parameter of type 'Context & Record<"mySecret", any>'.  
Property 'mySecret' is missing in type 'Context' but required in type 'Record<"mySecret", any>'

The test code looks like this:

import {Context as LambdaContext} from "aws-lambda";
import {handler} from "./handler.js";

const context: LambdaContext = {
    awsRequestId: 'testId',
    callbackWaitsForEmptyEventLoop: false,
    functionName: 'test-lambda',
    functionVersion: '1',
    invokedFunctionArn: 'a',
    memoryLimitInMB: '1',
    logGroupName: 'myloggroup',
    logStreamName: 'mystream',
    getRemainingTimeInMillis: () => 1,
    done() {
        /* deprecated method that is required in the type */
    },
    fail() {
        /* deprecated method that is required in the type */
    },
    succeed() {
        /* deprecated method that is required in the type */
    }
}

await handler({}, context);

We can put a // @ts-expect-error comment here but it would be great not to have to use this!

[lmammino]

Hello, sorry for the delay in taking action on this. Finally having some time to dedicate to middy.

Based on what has been discussed so far in this thread, I believe this is an actionable list of tasks that we could realistically try to tackle (hopefully in time for v5):

• TypeScript improvement: getInternal type safety #1084 (suggested by @m-radzikowski)
• TypeScript improvement: @middy/http-json-body-parser should support API Gateway V2 events #1085 (suggested by @apalchys)
• TypeScript improvement: Improve documentation to highlight _middleware-first, handler-last_ usage #1086 (suggested by @bilalq)
• TypeScript improvement: Replace any with unknown where it makes sense #1087 (suggested by @bilalq)
• TypeScript improvement: Context should keep track of changes made by middleware(s) #1088 (suggested by @ben-eb)

I opened dedicated issues for those, so we can send specific PRs for each one of these tasks, have more focused conversations, and track the progress on individual tasks.

I'll try my best (with my limited availability and my poor TypeScript knowledge) to start to tackle them, but more than happy to delegate some of these tasks to volunteers if anyone is interested in taking ownership.

Let me know if this approach and the list of tasks make sense to you. I am more than happy to reconsider this plan and take a different path. I'd love to keep this as a community-driven effort! 😊

[lmammino]

It was also mentioned to reconsider #914 (originally closed for inactivity). Should I add that to the list as well?

[GentileFulvio]

Wouldn't it make more sense to rewrite all javascript files in actual typescript ? This seems like the best way to make sure types are respected in the code as well

[lmammino]

Wouldn't it make more sense to rewrite all javascript files in actual typescript ? This seems like the best way to make sure types are respected in the code as well

Hey @GentileFulvio, my 2 cents is that, in general, it would make sense, but in practice, the maintainers team doesn't have a hell of a lot of experience with TS and we have found that most other contributors don't have much experience either...

For now, we want to keep focusing on providing a strong JavaScript experience while the TS experience is more of a best-effort thing (and we rely heavily on the community to tell us what's lacking and how to improve things). If, eventually, we get some core contributor with a good amount of expertise in TS we might re-evaluate this decision and we might go full-on with TS.

[willfarrell]

All related issues are now merged into the v5 branch. Closing.