aws / aws-appsync-community

The AWS AppSync community
https://aws.amazon.com/appsync
Apache License 2.0
506 stars 32 forks source link

Type checking blocking deployment #334

Closed thscott closed 4 weeks ago

thscott commented 10 months ago

I've stumbled across behaviour in appsync js resolvers that doesn't seem to be documented anywhere (though it is mentioned in this article.

It appears that as part of the validation process, type checking is done (of the form you'd get if you added // @ts-check to the top of a js file).

This leads to issues with code such as:

let b = [{"a": 5}];
b.push({"b": 6});

which results in the error Argument of type '{ b: number; }' is not assignable to parameter of type '{ a: number; }'. Property '"a"' is missing in type '{ b: number; }' but required in type '{ a: number; }'

A workaround for this is to add a jsdoc comment specifying the desired type:

/** @type {Record<string, number>[]} */
let b = [{"a": 5}];
b.push({"b": 6});

This is however not able to be done when using esbuild to transform typescript to javascript (as recommended in the docs), as esbuild strips out such comments.

There is usually a way to work around this, such as:

let b = []; // type is now any[]
b.push({"a": 5});
b.push({"b": 6});

but this is a pain to do for what is actually valid code.

Would it be possible to either disable these typechecks, and/or support typescript directly so that the correct types can be specified.

gilbertw1 commented 9 months ago

Thanks for reporting this. We are currently investigating the issue and will follow up when we have an update.

patrik-simunic-cz commented 9 months ago

@gilbertw1 Do I understand it correctly that you did not intent this behaviour? I have had to be doing things like this for months:

Screenshot 2023-11-26 at 8 08 44
saltman424 commented 6 months ago

@gilbertw1 Any update from the investigation?

I am encountering this issue repeatedly since I am using TypeScript with esbuild, which means sometimes generated JavaScript does not yield inferable types

thscott commented 1 month ago

Came back to working with AppSync resolvers recently, and almost immediately ran into this again.

I had code similar to:

function pickDefined<T extends Record<string, any>>(obj: T) {
    return (Object.keys(obj) as (keyof T)[]).reduce((outObj, key) => {
        const val = obj[key];
        if (val !== undefined && val !== null) outObj[key] = val;
        return outObj;
    }, {} as NonNullableRecord<T, keyof T>);
}

export function request(ctx: Context<SomeType>) {
  const config = pickDefined(ctx.args.config);
  const aProp = config.aProp;
}

This was rejected by AppSync because when types are stripped, pickDefined becomes:

function pickDefined(obj) {
  return Object.keys(obj).reduce((outObj, key) => {
    const val = obj[key];
    if (val !== void 0 && val !== null) outObj[key] = val;
    return outObj;
  }, {});
}

which has a return type of {}.

Therefore accessing any properties on it is a type error!

I worked around this by forcing config to be of any type (after type stripping):

export function request(ctx: Context<SomeType>) {
  let config = JSON.parse("{}") as ReturnType<typeof pickDefined<typeof ctx.args.config>>;
  config = pickDefined(ctx.args.config);
  const aProp = config.aProp;
}

It's utterly absurd that AppSync basically requires typed JavaScript in the form of JSDoc comments, but doesn't accept raw TypeScript. The type checking either needs to be disabled, or AppSync needs to accept TypeScript and do the type stripping itself, after checking for type errors.

Has there been any progress or investigation into this? @gilbertw1 @onlybakam

At the very least, this issue should be in the documentation somewhere, and ideally a nicer error message from CloudFormation would be returned. Currently all you get is:

The code contains one or more errors. (Service: AppSync, Status Code: 400, Request ID ....

gilbertw1 commented 1 month ago

@thscott Hey, thanks for reporting this issue.

There has been some progress on this front recently. The previous two reported issues have been resolved and the associated code samples now validate correctly.

We're currently investigating a fix for the specific issue you're encountering and I expect it will be resolved shortly. I'll follow-up here once it is.

gilbertw1 commented 4 weeks ago

Following up here, this issue has been resolved and each of the code samples mentioned in these comments should no longer result in failure when creating functions or resolvers.