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

New terminology #9

Merged
merged 7 commits into from
Oct 16, 2017
Merged

New terminology #9

merged 7 commits into from
Oct 16, 2017

Conversation

schickling
Copy link
Contributor

@schickling schickling commented Oct 13, 2017

With many terminology changes being introduced in /~https://github.com/graphcool/graphcool, graphcool-lib also needs to be updated.

The following library changes have been introduced:

  • Rename projectId to serviceId
  • Rename pat to rootToken
  • Rename generateUserToken to generateNodeToken
  • Add createSchema(): Promise<GraphQLSchema> to simplify API gateway code (cc @nikolasburk)

Changes required from the backend

      mutation {
        generateNodeToken(input: {
          rootToken: "${this.rootToken}"
          serviceId: "${this.serviceId}"
          nodeId: "${nodeId}", 
          modelName: "${typeName}",
          expirationInSeconds: Int
          clientMutationId: "static"
        }) {
          token
        }
      }

Notes

  • Be careful when introducing breaking changes since the current function runtime (being based on Auth0 Extend) always pulls in the latest version of a package, so a redeploy of a function could break a customer's app
  • Consider bumping the version count to 1.x (see prev point)
  • When merging this please add an extensive README & release notes

@kbrandwijk
Copy link

kbrandwijk commented Oct 13, 2017

You have kept the tokenOrRootToken method, but at the same time, the rename caused the behavior to change, and a single token to be used.

fromEvent used to set the pat (now rootToken), api() used to set to token when specified in the options. However, there is no rootToken anywhere. I proposed going forward with one token, but then the tokenOrRootToken method is no longer needed.

@kbrandwijk
Copy link

Another question: can you restructure the code so the new createSchema method goes into a different class? Because now whenever you require graphcool-lib, you're also pulling all these extra dependencies in (apollo-link-http, graphql, graphql-tools), making this lightweight module a lot heavier then it needs to be, because the createSchema functionality is very specific, and will never be used in functions.

@kbrandwijk
Copy link

I like the fact that parameter naming is consistent now. Just token. However, as generateNodeToken specifically requires a rootToken, there should be an additional check to see if the provided token is a rootToken or not.

@timsuchanek timsuchanek merged commit 378e940 into master Oct 16, 2017
@kbrandwijk
Copy link

kbrandwijk commented Oct 18, 2017

@timsuchanek The release notes don't mention the biggest breaking change: having to specify endpoints manually outside resolver functions!

@mwickett
Copy link

@kbrandwijk Can you elaborate on that change?

@kbrandwijk
Copy link

@mwickett Yes, I created #14 for it, but it is already fixed by @timsuchanek.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants