Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aws-appsync: javascript resolver support #22921

Closed
1 of 2 tasks
mtliendo opened this issue Nov 15, 2022 · 38 comments · Fixed by #23551
Closed
1 of 2 tasks

aws-appsync: javascript resolver support #22921

mtliendo opened this issue Nov 15, 2022 · 38 comments · Fixed by #23551
Labels
@aws-cdk/aws-appsync Related to AWS AppSync effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2

Comments

@mtliendo
Copy link

Describe the feature

Based on this RFC, it would nice to explore what a CDK supported candidate would look like.

Use Case

I'd like the experimental L2 construct to support the JS runtime for AppSync resolvers

Proposed Solution

the datasource.createResolver() method should accept a code argument, a specified runtime, and make the mappingTemplates optional.

From this:

const appsyncFunction = new appsync.AppsyncFunction(this, 'function', {
  name: 'appsync_function',
  api,
  dataSource: api.addNoneDataSource('none'),
  requestMappingTemplate: appsync.MappingTemplate.fromFile('request.vtl'),
  responseMappingTemplate: appsync.MappingTemplate.fromFile('response.vtl'),
});

const pipelineResolver = new appsync.Resolver(this, 'pipeline', {
  api,
  dataSource: api.addNoneDataSource('none'),
  typeName: 'Query',
  fieldName: 'getDemos',
  requestMappingTemplate: appsync.MappingTemplate.fromFile('beforeRequest.vtl'),
  pipelineConfig: [appsyncFunction],
  responseMappingTemplate: appsync.MappingTemplate.fromFile('afterResponse.vtl'),
});

To this:

const appsyncFunction = new appsync.AppsyncFunction(this, 'function', {
  name: 'appsync_function',
  api, 
  dataSource: api.addNoneDataSource('none'),
  runtime:  appsync.Runtime.APPSYNC_JS,
  code: appsync.Code.fromAsset(path.join(__dirname, 'fn1.js'))
});

const pipelineResolver = new appsync.Resolver(this, 'pipeline', {
  api,
  dataSource: api.addNoneDataSource('none'),
  typeName: 'Query',
  fieldName: 'getDemos',
  code: appsync.Code.fromAsset(path.join(__dirname, 'pipelineMappings.js'))
  pipelineConfig: [appsyncFunction],
});

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.50.0

Environment details (OS name and version, etc.)

macOS

@mtliendo mtliendo added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Nov 15, 2022
@github-actions github-actions bot added the @aws-cdk/aws-appsync Related to AWS AppSync label Nov 15, 2022
@peterwoodworth
Copy link
Contributor

We're bound by the CloudFormation API which requires a template within the CloudFormation template, or requires the template to reside in S3. I'm not sure how exactly we'd be able to implement this feature, or how this would be useful given the CloudFormation requirements. Could you explain exactly how this would be useful and how we might be able to accomplish this feature?

@peterwoodworth peterwoodworth added p2 response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Nov 15, 2022
@garretcharp
Copy link

Not really sure how cloudformation actually works but this would be the absolute most useful feature for appsync. VTL is really annoying to work with compared to javascript

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Nov 18, 2022
@mtliendo
Copy link
Author

We on the AppSync team would be happy to help push this PR through based on our new release today 😊

https://aws.amazon.com/blogs/aws/aws-appsync-graphql-apis-supports-javascript-resolvers/

@piotrmoszkowicz
Copy link
Contributor

According to https://github.com/aws/aws-cdk/blob/04baba88b4e27e19b7d0ab237ff641043d089edc/packages/%40aws-cdk/cfnspec/spec-source/specification/000_cfn/000_official/000_AWS_AppSync.json which is currently being merged, there's Code, CodeS3Location and Runtime (with Name and RuntimeVersion properties), which could be used for that. I assume we can implement it. I can volunteer for that, but could do Sunday / beginning of next week, but would love to help!

@piotrmoszkowicz
Copy link
Contributor

Also for Code - we could partially take a look on https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_lambda.Code.html and copy part of it (ie. fromInline, fromAsset, fromDockerImage and fromBucket). Also would be nice to take a look at https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_lambda_nodejs-readme.html and enable bundling via esbuild - I would love to have that feature!

@fab-mindflow
Copy link

👍 strongly interested in this support at our startup already heavily using AppSync w/ CDK.

@mtliendo
Copy link
Author

mtliendo commented Nov 18, 2022

@piotrmoszkowicz I have a CDK repo up (can finally make public 😅 ) that uses the L1 construct in conjunction with the addOverrides property to enable support for JS resolvers. The overrides are only needed due to the latest CloudFormation updates not being merged into the CDK yet (I think this may happen some time next week)

The main points to call out are when I create a function, add the overrides and then create create the pipeline, then and those overrides.

Would you be interested in making this contribution to the L2 alpha construct?

@piotrmoszkowicz
Copy link
Contributor

@mtliendo Yes I will be interested! Could you pass me possible values for runtime (both name and version)? As CF docs are not yet published I can't look it up 😅

@mtliendo
Copy link
Author

APPSYNC_JS is a string and the only runtime at the moment, but should be an enum to support other things that may come later 😉

'1.0.0' for the version. It's represented as a string and should stay a string.

Also, as seen in the repo, JS files have to be passed as string, so I'm using fs.readFile but if there's a more elegant way (like how the graphql file gets turned into a string) then that would be great to have.

You're awesome, thanks so much for helping to bring this in and please let me know how I can help!

@piotrmoszkowicz
Copy link
Contributor

@mtliendo So we just keep Runtime Name as ENUM, how about VTL? Because I would love to have statically typed solution, which basically tells you to specify runtime when you specify code and same approach for VTL 😊 I guess for VTLs nothing have change, yes?

@mtliendo
Copy link
Author

You got it :) For VTL the process is the same. JS resolver support is opt-in by specifying a JS file to the Code property.

But your right, it would be nice to get type checking on that 🤔 I could be wrong, but this may actually be a good use case for the never keyword based off of what I've seen others do.

@otaviomacedo otaviomacedo removed their assignment Nov 18, 2022
@mtliendo
Copy link
Author

Cloudformation docs are updated :) Im told an automated PR should be available in less than a week to update the L1 construct.

@peterwoodworth peterwoodworth added the effort/medium Medium work item – several days of effort label Nov 18, 2022
@piotrmoszkowicz
Copy link
Contributor

Awesome, will check that!

@riywo
Copy link

riywo commented Nov 30, 2022

Any chance to get this update? L1 is just fine for me.

@piotrmoszkowicz
Copy link
Contributor

I hope to work on that during this weekend :)

@MrArnoldPalmer
Copy link
Contributor

L1 support is landed and myself and @mtliendo are working on an implementation for the L2s. PRs also welcome if anyone gets to it in the meantime.

@mattiLeBlanc
Copy link

mattiLeBlanc commented Dec 13, 2022

Firstly, thank you for this implementation of this great new improvement of Appsync.

Can anyone give some examples for L1 in TS code for adding a resolver?
And also for L2 when ready?
I assume it will take some time for docs to be updated.

@mtliendo
Copy link
Author

@mattiLeBlanc keep in mind that it’s still possible to use the L2 + the escape hatches: https://github.com/mtliendo/reinvent-chat/tree/main/lib/graphql

@mattiLeBlanc
Copy link

@mtliendo Wonderful! Thank you so much.

@mattiLeBlanc
Copy link

@mtliendo So just to be clear, is this production ready or should I rely on VTL resolvers for now and wait for this to mature a bit more?

@mtliendo
Copy link
Author

mtliendo commented Dec 13, 2022

JS resolvers shipped production ready with no loss in functionality or increase in latency 🙂

the L1 construct is updated however many folks use the L2 construct ( rightfully so), and that’s what this weeks PR will be targeting.

@mergify mergify bot closed this as completed in #23551 Jan 6, 2023
mergify bot pushed a commit that referenced this issue Jan 6, 2023
Adds support for resolvers using the APPSYNC_JS runtime. Adds new props
for specifying `Code` and `Runtime` on both `Resolver` and `Function`
constructs. Adds `Code` class with asset support for local files and
inline code.

Adds integ test covering basic JS function/resolver usage.

Fix: #22921
@github-actions
Copy link

github-actions bot commented Jan 6, 2023

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues 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.

@mattiLeBlanc
Copy link

@mtliendo Hi, thanks for you hard work!
Can you explain to me the difference with L1 and L2 constructs? Which one are the higher level ones?
I one of the examples (https://github.com/mtliendo/reinvent-chat/blob/main/lib/graphql/utils/index.ts) you reference L1 constructs as an escape hatch and having to define the JS Runtime. It looks like L1 is the lower level construct and we should use the higher level, new AppsyncFunction() ? right?

@MrArnoldPalmer
Copy link
Contributor

@mattiLeBlanc L2s are higher level than L1s. L1s are direct 1:1 representations of the underlying cloudformation resources. Escape hatches are a mechanism for usually used to manipulate the L1 constructs that are composed within an L2.

Here are some docs explaining the relevant concepts:
constructs
escape hatches

L2s are generally recommended but they can be opinionated so if you disagree with those built in opinions, L1s can sometimes be better for your use case. In the case of AppSync though the L2s don't really layer in too much beyond just convenience, IE bundling your resolver code etc.

@danrivett
Copy link

danrivett commented Mar 30, 2023

Following on from a great article from @bboure about a possible way to share common local functions across multiple JavaScript resolvers using esbuild, I was wondering how feasible it is to do with a CDK deployed Javascript Resolver?

It would be fantastic to support this, as it's really the main missing piece I am facing. I'm currently copying and pasting common functions between resolvers as I'm not sure how else to automate running esbuild on a cdk deploy (#17377 ?).

Given Lambda functions (NodejsFunction) has built in support to configure esbuild bundling, I'm hopefuly my request isn't too far off base, but I welcome alternative suggestions.

Update: I see #24548 has been created already which looks to have raised this question already.

@bboure
Copy link

bboure commented Mar 31, 2023

Thanks @danrivett for the mention and reference.
While this isn't supported in the CDK officially, I have a follow up article that I will publish hopefully soon (it's not quite ready yet) that covers this.

TL;DR; you can bundle the code yourself (That's an example with TypeScript, but it should work just fine with JS too)

@danrivett
Copy link

danrivett commented Mar 31, 2023

That's fantastic @bboure, thank you so much for pointing me to your example, that is really helpful. I'll give it a go later today and also look forward to your blog in the future.

We largely use TypeScript over JavaScript, but I reverted to JavaScript for our AppSync JavaScript resolvers as I wasn't clear how best to support transpiling TS to JS via our CDK-managed stack.

Your solution looks really simple. I see it uses Code.fromInline(). I think that has a limitation that the code is maximum 4KiB in size (see here).

That likely isn't a huge issue, but I wondered if there is a simple way to use Code.fromAsset() instead? I assume just having esbuild write to a local dist directory and referencing that would be all that's needed.

In that case I hope your example will also provide inspiration to AWS to to officially support this, as it would be great to simplify and standardize that support because I think it would really super-charge AppSync JavaScript resolvers adding both local imports and TypeScript support.

Perhaps they could add Code.buildAsset(filePath, esBuildOptions), that would be great.

@bboure
Copy link

bboure commented Mar 31, 2023

Thanks @danrivett I wasn't aware of this limit.

If this becomes an issue, an alternative it to pre-build the resolvers and use fromAsset(), yes.

esbuild src/*.ts \
  --bundle \
  --outdir=build \
  --external:"@aws-appsync/utils" \
  --format=esm
  --target=es2020

then point fromAsset to the build directory.

 code: Code.fromAsset('build/resolver.js'),

tip: change cdk.json to

"app": "esbuild src/*.ts ..... && npx ts-node --prefer-ts-exts --project=tsconfig.json ./src/index.ts ",

@lysachok
Copy link

any ideas why this approach (code: Code.fromAsset('build/resolver.js')) leads to the error:

The specified bucket does not exist (Service: Amazon S3; Status Code: 404; Error Code: NoSuchBucket;

?

@MrArnoldPalmer
Copy link
Contributor

@lysachok usually this is caused by the account/region not being bootstrapped.
https://docs.aws.amazon.com/cdk/v2/guide/bootstrapping.html

@lysachok
Copy link

@lysachok usually this is caused by the account/region not being bootstrapped.

https://docs.aws.amazon.com/cdk/v2/guide/bootstrapping.html

@MrArnoldPalmer right, but I was trying to do that in an Amplify project so it didn't help anyway.

Thank you for replying!

@mattiLeBlanc
Copy link

That's fantastic @bboure, thank you so much for pointing me to your example, that is really helpful. I'll give it a go later today and also look forward to your blog in the future.

We largely use TypeScript over JavaScript, but I reverted to JavaScript for our AppSync JavaScript resolvers as I wasn't clear how best to support transpiling TS to JS via our CDK-managed stack.

Your solution looks really simple. I see it uses Code.fromInline(). I think that has a limitation that the code is maximum 4KiB in size (see here).

That likely isn't a huge issue, but I wondered if there is a simple way to use Code.fromAsset() instead? I assume just having esbuild write to a local dist directory and referencing that would be all that's needed.

In that case I hope your example will also provide inspiration to AWS to to officially support this, as it would be great to simplify and standardize that support because I think it would really super-charge AppSync JavaScript resolvers adding both local imports and TypeScript support.

Perhaps they could add Code.buildAsset(filePath, esBuildOptions), that would be great.

This brilliant example also has an sync esbuild during CDK deployment:
https://github.com/graphboltdev/appsync-typescript-resolvers/blob/master/lib/helpers.ts
https://github.com/graphboltdev/appsync-typescript-resolvers/blob/master/lib/cdk-stack.ts

@mattiLeBlanc
Copy link

mattiLeBlanc commented Jul 12, 2023

@MrArnoldPalmer
Hi,

I am trying to implement support for JS resolvers into my Appsync CDK script.
I am having a lot of issues converting my Typescript templates to Javascript and make them acceptable to the CDK.
Long story short, AmazonDeepdish is giving me a hard time on code: The code contains one or more errors. (Service: AmazonDeepdish; Status Code: 400;

I tried multiple things like building the TS in my dist but it becomes es6 (with arrow functions etc) and AmazonDeepdish doesn't like it.
Then I tried the esbuild solution, to convert the TS into JS on the fly, but esbuild doesn't support Es5 anymore (to get rid of imports and arrow functions). Now I am running @swc's transformSync like

    module: {
      type: 'commonjs'
    },
    jsc: {
      parser: {
        syntax: 'typescript',

      },
      target: 'es5',
    }
  });

which I hope is acceptable code, but still Deepdish doesn;t like it.

Code generated by CDK Synth:

Name: testFacility
      Code: |
        "use strict";
        Object.defineProperty(exports, "__esModule", {
            value: true
        });
        var _utils = require("@aws-appsync/utils");
        function _class_call_check(instance, Constructor) {
            if (!(instance instanceof Constructor)) {
                throw new TypeError("Cannot call a class as a function");
            }
        }
        function _defineProperties(target, props) {
            for(var i = 0; i < props.length; i++){
                var descriptor = props[i];
                descriptor.enumerable = descriptor.enumerable || false;
                descriptor.configurable = true;
                if ("value" in descriptor) descriptor.writable = true;
                Object.defineProperty(target, descriptor.key, descriptor);
            }
        }
        function _create_class(Constructor, protoProps, staticProps) {
            if (protoProps) _defineProperties(Constructor.prototype, protoProps);
            if (staticProps) _defineProperties(Constructor, staticProps);
            return Constructor;
        }
        var Resolver = /*#__PURE__*/ function() {
            "use strict";
            function Resolver() {
                _class_call_check(this, Resolver);
            }
            _create_class(Resolver, null, [
                {
                    key: "request",
                    value: function request(ctx) {
                        return {
                            operation: "Query",
                            query: {
                                expression: "sk = :sk",
                                index: "sk-pk-index",
                                expressionValues: {
                                    ":sk": _utils.util.dynamodb.toDynamoDB("FACILITY:COUNTRY=".concat(ctx.args.country))
                                }
                            }
                        };
                    }
                },
                {
                    key: "response",
                    value: function response(ctx) {
                        console.log(ctx.args);
                        return ctx.result;
                    }
                }
            ]);
            return Resolver;
        }();
        exports.request = Resolver.request;
        exports.response = Resolver.response;
      FunctionVersion: "2018-05-29"
      Runtime:
        Name: APPSYNC_JS
        RuntimeVersion: 1.0.0
    DependsOn:

The TS source is:

import { Context, util } from '@aws-appsync/utils';
import { ResolverConfig } from '../../../../lib/helpers/appsync.helper';

@ResolverConfig({
  name: 'testFacility',
  datasource: 'DYNAMODB',
  datasourceName: 'facilities',
})
class Resolver {
  static request(ctx: Context<any>) {
    return {
      operation: 'Query',
      query: {
        expression: 'sk = :sk',
        index: 'sk-pk-index',
        expressionValues: {
          ':sk': util.dynamodb.toDynamoDB(`FACILITY:COUNTRY=${ctx.args.country}`),
        },
      },
    };
  }
  static response(ctx: Context<any, object, object, object, any>,) {
    console.log(ctx.args);
    return ctx.result;
  }
}
exports.Resolver = Resolver;
exports.request = Resolver.request;
exports.response = Resolver.response;

Yes, I know I am using a class, but that is because I try to use the Decorator to pass in config information for the Resolver (Angular style). I filter out the Decorator and its import before I convert to es5.
It works, but maybe Deepdish doesn't like classes?
Can you tell me what is wrong with my JS atm.
Maybe I have to put the config in a comment block and parse that instead of using Class+Decorator.... sigh.

I must say, I like the JS support, but I find it very cumbersome;

  1. you can't create a JS unit resolver, but it always have to be a pipeline resolver with a function resolver inside
  2. JS support seems to be poor, how great would it be if we can just use Typescript from the start and have a helper function convert it using @swc or some other library.

I have to park my JS resolver implementation because it completely derailed me. I just got so excited.

@bboure
Copy link

bboure commented Jul 12, 2023

@mattiLeBlanc APPSYNC_JS is very strict and you can't do many of the things you are doing. I recommend that you refer to the documentation and check the supported features (e.g. Classes are not supported)

I also advise that you use the @aws-appsync/eslint-plugin npm package to validate your JS (see here)

Finally, you can also use the evaluate-code command to test and get better feedback about errors than CloudFormation. Alternatively, GraphBolt also has a tool to evaluate code in a GUI 😛

@hoegertn
Copy link
Contributor

Here is some code I am using to bundle AppSync functions: https://github.com/taimos/cdk-serverless/blob/main/src/constructs/graphql.ts#L279

Beware that I name the js files .jar to avoid a missing feature in CDK assets I will address here: #26106

@sudokar
Copy link

sudokar commented Jul 30, 2023

Built a custom AWS CDK construct to transpile and bundle Typescript to Javascript https://github.com/sudokar/cdk-appsync-typescript-resolver

@samturner3
Copy link

One feature I feel is missing is being able to provide a dynamic value to the resolver from the cdk file.
See: #26848

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-appsync Related to AWS AppSync effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.