Maintenance: add biome to project (parser)

[dreamorosi]

Summary

With #2791 being merged, we can now migrate the packages/parser workspace to use Biome.

As part of the migration I will fix any issue/inconsistency that arises. I'll do my best to minimize changes and leave the logic untouched.

Why is this needed?

So that we can gradually replace eslint, prettier, and all their dependencies with a single one, @biomejs/biome.

Which area does this relate to?

Other

Solution

No response

Acknowledgment

• This request meets Powertools for AWS Lambda (TypeScript) Tenets
• Should this be considered in other Powertools for AWS Lambda languages? i.e. Python, Java, and .NET

Future readers

Please react with 👍 and your use case to help us understand customer demand.

[daschaa]

I would like to take this issue :)

[dreamorosi]

I think there will be some challenges with the conversion, especially with the classes within the packages/parser/src/envelopes, like this one.

All the classes are defined more or less like this:

export class ApiGatewayV2Envelope extends Envelope {
  public name = 'ApiGatewayV2Envelope';
  public static parse<T extends ZodSchema>(
    data: unknown,
    schema: T
  ): z.infer<T> {
    return Envelope.parse(
      APIGatewayProxyEventV2Schema.parse(data).body,
      schema
    );
  }

  public static safeParse<T extends ZodSchema>(
    data: unknown,
    schema: T
  ): ParsedResult {
    const parsedEnvelope = APIGatewayProxyEventV2Schema.safeParse(data);
    if (!parsedEnvelope.success) {
      return {
        success: false,
        error: new ParseError('Failed to parse API Gateway V2 envelope', {
          cause: parsedEnvelope.error,
        }),
        originalEvent: data,
      };
    }

    const parsedBody = Envelope.safeParse(parsedEnvelope.data.body, schema);

    if (!parsedBody.success) {
      return {
        success: false,
        error: new ParseError('Failed to parse API Gateway V2 envelope body', {
          cause: parsedBody.error,
        }),
        originalEvent: data,
      };
    }

    return parsedBody;
  }
}

which according to Biome has two issues:

1. The methods use the super. keyword within the static methods
2. The class itself only declares static methods

The first one is easily fixable thanks to the fact that the methods being called in the parent class are themselves static, so we can just replace all the occurrences of super.parse() with Envelope.parse() (and the same with safeParse).

The second one is a bit trickier. The way I see it, we have three options:

• suppress the warnings
• add some other property to the class (i.e. public name = 'ApiGatewayV2Envelope')
• refactor the code and follow the suggestion from Biome

I'd like to avoid the former unless we have no other option, and the second seems useless. So let's explore the third.

These classes were created like that to allow customers to use them without having to instantiate them. The use case we want to enable is people doing ApiGatewayV2Envelope.parse(...) rather than new ApiGatewayV2Envelope().parse(...), which is sensible.

To maintain this functionality and also remove the warnings, we could refactor both classes to plain object, and set the two methods as object properties. Once refactored they would become this:

export const Envelope = {
  /**
   * Abstract function to parse the content of the envelope using provided schema.
   * Both inputs are provided as unknown by the user.
   * We expect the data to be either string that can be parsed to json or object.
   * @internal
   * @param data data to parse
   * @param schema schema
   */
  parse: <T extends ZodSchema>(
    data: unknown,
    schema: T
  ): z.infer<T> => {
    if (typeof data !== 'object' && typeof data !== 'string') {
      throw new ParseError(
        `Invalid data type for envelope. Expected string or object, got ${typeof data}`
      );
    }
    try {
      if (typeof data === 'string') {
        return schema.parse(JSON.parse(data));
      }
      if (typeof data === 'object') {
        return schema.parse(data);
      }
    } catch (error) {
      throw new ParseError('Failed to parse envelope', {
        cause: error as Error,
      });
    }
  },

  /**
   * Abstract function to safely parse the content of the envelope using provided schema.
   * safeParse is used to avoid throwing errors, thus we catuch all errors and wrap them in the result.
   * @param input
   * @param schema
   */
  safeParse: <T extends ZodSchema>(
    input: unknown,
    schema: T
  ): ParsedResult<unknown, z.infer<T>> => {
    try {
      if (typeof input !== 'object' && typeof input !== 'string') {
        return {
          success: false,
          error: new ParseError(
            `Invalid data type for envelope. Expected string or object, got ${typeof input}`
          ),
          originalEvent: input,
        };
      }

      const parsed = schema.safeParse(
        typeof input === 'string' ? JSON.parse(input) : input
      );

      return parsed.success
        ? {
            success: true,
            data: parsed.data,
          }
        : {
            success: false,
            error: new ParseError('Failed to parse envelope', {
              cause: parsed.error,
            }),
            originalEvent: input,
          };
    } catch (error) {
      return {
        success: false,
        error: new ParseError('Failed to parse envelope', {
          cause: error as Error,
        }),
        originalEvent: input,
      };
    }
  }
}

export const ApiGatewayEnvelope = {
  parse<T extends ZodSchema>(
    data: unknown,
    schema: T
  ): z.infer<T> {
    return Envelope.parse(APIGatewayProxyEventSchema.parse(data).body, schema);
  },

  safeParse<T extends ZodSchema>(
    data: unknown,
    schema: T
  ): ParsedResult<unknown, z.infer<T>> {
    const parsedEnvelope = APIGatewayProxyEventSchema.safeParse(data);
    if (!parsedEnvelope.success) {
      return {
        success: false,
        error: new ParseError('Failed to parse ApiGatewayEnvelope', {
          cause: parsedEnvelope.error,
        }),
        originalEvent: data,
      };
    }

    const parsedBody = Envelope.safeParse(parsedEnvelope.data.body, schema);

    if (!parsedBody.success) {
      return {
        success: false,
        error: new ParseError('Failed to parse ApiGatewayEnvelope body', {
          cause: parsedBody.error,
        }),
        originalEvent: data,
      };
    }

    return parsedBody;
  }
}

By doing this the logic would remain the same, but the resulting code would also be slightly simpler. Additionally, when bundling the current Parser code using esbuild, this is exactly the same transformation that is applied, so we'd get to this anyway at runtime.

What do you think - cc @am29d?

[daschaa]

Seeing this, I'm even thinking about whether there are reasons to keep the envelope classes as classes. I can't see any advantage why they should stay as a class, right?

But is it a breaking change? In theory the envelope classes are instantiable right? So if a customer (for whatever reason) instantiated this, a migration from class to object would break their code right? 🤔

[dreamorosi]

Both are can be instantiated, but in my opinion neither of them has to be a class strictly speaking.

Regarding the breaking change, normally it would be one, but in this instance we have two points to consider:

• the utility is still in beta and we don't guarantee stability until it's considered as generally available (next release)
• all documentation shows how to use the object as static (aka without the new keyword)

I think overall if we want to make a change like this it's now or never for the next couple years

[am29d]

The refactoring to objects makes sense, there is nothing class specific in the implementation. I have checked and there is no impact on types or other parts of the parser, tests are green.

[github-actions]

⚠️ COMMENT VISIBILITY WARNING ⚠️

This issue is now closed. Please be mindful that future comments are hard for our team to see.

If you need more assistance, please either tag a team member or open a new issue that references this one.

If you wish to keep having a conversation with other community members under this issue feel free to do so.