aws-amplify / amplify-category-api

The AWS Amplify CLI is a toolchain for simplifying serverless web and mobile development. This plugin provides functionality for the API category, allowing for the creation and management of GraphQL and REST based backends for your amplify project.
https://docs.amplify.aws/
Apache License 2.0
89 stars 77 forks source link

Incorrect GraphQL Transformer v2 Documentation - Leading to Poor Initial DX #102

Closed danrivett closed 8 months ago

danrivett commented 2 years ago

I'm currently migrating our Amplify project over from the v1 to v2 GraphQL Transformer and got stuck migrating our previous customized resolvers to the new pipeline resolver model with the following error:

An error occurred during the push operation: Resolver is missing slot preDataLoad

I followed the documentation here and noted the example and listed supported resolver slots section and so chose the preDataLoad as the appropriate place to put our customization and so created the following file:

Mutation.createUser.preDataLoad.1.req

under the resolvers directory. As mentioned above I got the following error:

✖ An error occurred when pushing the resources to the cloud
🛑 An error occurred during the push operation: Resolver is missing slot preDataLoad
⚠️ Please refer Amplify CLI troubleshooting guide at : https://docs.amplify.aws/cli/project/troubleshooting/

Reading the linked troubleshooting page didn't help, nor did googling for a solution.

The only way I found what I believe is solution, was to download the source code and search for the string Resolver is missing slot.

This located this file which after reading through the source code showed that for a GraphQL Mutation preDataLoad is not valid, only the following resolver slots are:

      ['init', 'preAuth', 'auth', 'postAuth', 'preUpdate'],
      ['postUpdate', 'finish'],

It looks like preUpdate is analogous to preDataLoad for mutation operations.

Having a look at the commit history for this file, shows this doesn't appear to have changed in over a year, so there doesn't appear to be a great reason why the documentation isn't accurate, and I would have hoped it would have been found in review ahead of launch.

I'm creating this ticket to hopefully get the docs updated so that future users enjoy a better initial DX for the v2 Transformer migration as I'm sure the actual functionality is very good.

josefaidt commented 2 years ago

Hey @danrivett :wave: thanks for raising this and providing the feedback!

It looks like preUpdate is analogous to preDataLoad for mutation operations.

You are correct, this should be added to the supported resolver slots. It appears we have a few items here:

  1. improve the error messaging to inform of us what resolver is missing the slot, or provide a clearer direction
    🛑 An error occurred during the push operation: Resolver is missing slot preDataLoad
  2. update the supported resolver slots documentation with the different slots for Queries and Mutations
  3. improve the troubleshooting guide linked in the CLI output to cover the broad range of services (including relevant migration docs links) rather than only project-specific troubleshooting topics -- though this will require a more involved effort, in the immediate time frame we should update this to point to the reference doc for supported resolvers when encountering a resolver-related error
josefaidt commented 2 years ago

Marking this as a bug for the error message updates, and I will be linking the docs PR shortly 🙂

danrivett commented 2 years ago

Thanks for fixing the docs so quickly @josefaidt.

As a follow-up documentation improvement - feel free to create a separate bug to track if helpful - I'm struggling to find good documentation on how to migrate existing customizations of the old VTL resolver over to a new pipeline resolver.

In particular, I'm not finding enough documentation on the new context fields that the generated resolvers reference. For example:

etc...

I found the following doc which mentions the stash object, but it doesn't provide any details on what the standard resolvers populate it with, and so what custom pipeline resolvers slotted in can and should access.

It would make it much easier to create pipeliner resolvers if all the fields on the context object were listed and explained what they are used for and when to read/update them as necessary.

iyz91 commented 2 years ago

@josefaidt @danrivett I have to agree with Dan here, it seems the v2 transformer has introduced a fair number of breaking changes without accompanying documentation to guide the developer. I would say the documentation for v2 has a taken a few steps back vs v1 and I find it greatly reduces developer velocity.