Skip to content

Conversation

@yuth
Copy link
Contributor

@yuth yuth commented Oct 14, 2018

Updated the GraphQL API docs to use statements generated
by the codegen. Removed the amplify codegen add command as its
no longer neeed to be added

Updated the GraphQL API docs to use statements generated
by the codegen. Removed the `amplify codegen add` command as its
no longer neeed to be added
js/api.md Outdated
##### Type Generation using GraphQL Schemas

When working with GraphQL data, it is useful to import your types from your schema into your code for type safety. You can easily do this with Amplify CLI's automated code generation feature. The CLI automatically downloads GraphQL Introspection Schemas from your defined GraphQL endpoint and generates TypeScript or Flow classes for you.
When working with GraphQL data, it is useful to import your types from your schema into your code for type safety. You can easily do this with Amplify CLI's automated code generation feature. The CLI automatically downloads GraphQL Introspection Schemas from your defined GraphQL endpoint and generates TypeScript or Flow classes for you. Every time you push your GraphQL API, the cli will confirm if you want to generate the types and statements.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CLI in caps last sentence

js/api.md Outdated

You can declare GraphQL queries with standard GraphQL query syntax. To learn more about GraphQL queries, please visit [GraphQL Developer Documentation](http://graphql.org/learn/queries/).

Amplify cli codegen automatically generates all possible GraphQL statements (queries, mutations and subscriptions) and saves it in `src/graphql` folder.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...and for JavaScript applications saves it in src/graphql folder

Copy link
Contributor

@undefobj undefobj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yuth this looks good had a couple of minor comments. Also you made some changes to the connect components and used a Todo, which I like. Can you also incorporate the pending changes that I gave feedback for the Connect component on this PR: #11
@elorzafe was going to make them but perhaps you can incorporate or we can do two PRs?

@yuth yuth requested a review from elorzafe October 15, 2018 18:39
@yuth
Copy link
Contributor Author

yuth commented Oct 15, 2018

@undefobj I have incorporated the feedback from #11

Copy link
Contributor

@mlabieniec mlabieniec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple grammatical updates then LGTM

js/api.md Outdated

```
For mutations, a `mutation` function needs to be provided with `Connect` component. `mutation` returns a promise that resolves with the result of the GraphQL mutation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update to:

For mutations, a mutation function needs to be provided with the Connect component. A mutation returns a promise that resolves with the result of the GraphQL mutation.

js/api.md Outdated
##### Type Generation using GraphQL Schemas

When working with GraphQL data, it is useful to import your types from your schema into your code for type safety. You can easily do this with Amplify CLI's automated code generation feature. The CLI automatically downloads GraphQL Introspection Schemas from your defined GraphQL endpoint and generates TypeScript or Flow classes for you.
When working with GraphQL data, it is useful to import your types from your schema into your code for type safety. You can easily do this with Amplify CLI's automated code generation feature. The CLI automatically downloads GraphQL Introspection Schemas from your defined GraphQL endpoint and generates TypeScript or Flow classes for you. Every time you push your GraphQL API, the CLI will confirm if you want to generate the types and statements.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change to:

When working with GraphQL data it is useful to import types from your schema for type safety. You can do this with the Amplify CLI's automated code generation feature. The CLI automatically downloads GraphQL Introspection Schemas from the defined GraphQL endpoint and generates TypeScript or Flow classes for you. Every time you push your GraphQL API, the CLI will provide you the option to generate types and statements.

Copy link
Contributor

@elorzafe elorzafe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just needs a few fixes

js/api.md Outdated
// Query using a parameter
const oneEvent = await API.graphql(graphqlOperation(GetEvent, { id: 'some id' }));
const oneTodo = await API.graphql(graphqlOperation(queries.getTodo, { id: 'some id' }));
console.log(oneEvent);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to change oneEvent to oneTodo

js/api.md Outdated
};

const newEvent = await API.graphql(graphqlOperation(CreateEvent, eventDetails));
const newEvent = await API.graphql(graphqlOperation(mutations.createTodo, todoDetails));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switch newEvent to newTodo also
console.log(newTodo)

// Subscribe to creation of Todo
const subscription = API.graphql(
graphqlOperation(SubscribeToEventComments, { eventId: '123' })
graphqlOperation(subscriptions.onCreateTodo)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is missing the use case of sending params to the subscription I will mention that somewhere

js/api.md Outdated
<Connect query={graphqlOperation(queries.listTodos)}>
{({ data: { listTodos }, loading, error }) => {
if (error) return (<h3>Error</h3>);
if(loading || !listTodos) return (<h3>Loading...</h3>)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add an extra space on if (loading...

js/api.md Outdated
}

handleChange(name, ev) {
this.setState(name, ev.target.value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that is not going to work:
I would say something like:

this.setState({ [name]: ev.target.value });

Copy link
Contributor

@elorzafe elorzafe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! 👍

@mlabieniec mlabieniec closed this Oct 19, 2018
@mlabieniec mlabieniec reopened this Oct 19, 2018
@undefobj
Copy link
Contributor

Looks good to me - @elorzafe can you please confirm that we have the loading fix already deployed?

@kaustavghosh06 kaustavghosh06 merged commit 74367fc into aws-amplify:master Oct 23, 2018
@elorzafe
Copy link
Contributor

That is not merged yet, I was reviewing it with Manuel, we can merged and publish today

undefobj added a commit that referenced this pull request Dec 3, 2019
mlabieniec added a commit that referenced this pull request Feb 13, 2021
```
E/flutter (30435): [ERROR:flutter/lib/ui/ui_dart_state.cc(177)] Unhandled Exception: type '_InternalLinkedHashMap<String, dynamic>' is not a subtype of type 'Map<String, String>'
E/flutter (30435): #0      new CognitoSignUpOptions (package:amplify_auth_cognito/src/CognitoSignUp/CognitoSignUpOptions.dart:20:87)
E/flutter (30435): #1      SignUpState._signUp (package:AwsAmplifySample/components/signUp.dart:29:20)
E/flutter (30435): #2      SignUpState.build.<anonymous closure> (package:AwsAmplifySample/components/signUp.dart:190:35)
E/flutter (30435): #3      _InkResponseState._handleTap (package:flutter/src/material/ink_well.dart:993:19)
E/flutter (30435): #4      _InkResponseState.build.<anonymous closure> (package:flutter/src/material/ink_well.dart:1111:38)
E/flutter (30435): #5      GestureRecognizer.invokeCallback (package:flutter/src/gestures/recognizer.dart:183:24)
E/flutter (30435): #6      TapGestureRecognizer.handleTapUp (package:flutter/src/gestures/tap.dart:598:11)
E/flutter (30435): #7      BaseTapGestureRecognizer._checkUp (package:flutter/src/gestures/tap.dart:287:5)
E/flutter (30435): #8      BaseTapGestureRecognizer.acceptGesture (package:flutter/src/gestures/tap.dart:259:7)
E/flutter (30435): #9      GestureArenaManager.sweep (package:flutter/src/gestures/arena.dart:157:27)
E/flutter (30435): #10     GestureBinding.handleEvent (package:flutter/src/gestures/binding.dart:362:20)
E/flutter (30435): #11     GestureBinding.dispatchEvent (package:flutter/src/gestures/binding.dart:338:22)
E/flutter (30435): #12     RendererBinding.dispatchEvent (package:flutter/src/rendering/binding.dart:267:11)
E/flutter (30435): #13     GestureBinding._handlePointerEvent (package:flutter/src/gestures/binding.dart:295:7)
E/flutter (30435): #14     GestureBinding._flushPointerEventQueue (package:flutter/src/gestures/binding.dart:240:7)
E/flutter (30435): #15     GestureBinding._handlePointerDataPacket (package:flutter/src/gestures/binding.dart:213:7)
E/flutter (30435): #16     _rootRunUnary (dart:async/zone.dart:1206:13)
E/flutter (30435): #17     _CustomZone.runUnary (dart:async/zone.dart:1100:19)
E/flutter (30435): #18     _CustomZone.runUnaryGuarded (dart:async/zone.dart:1005:7)
E/flutter (30435): #19     _invoke1 (dart:ui/hooks.dart:265:10)
E/flutter (30435): #20     _dispatchPointerDataPacket (dart:ui/hooks.dart:174:5)
E/flutter (30435): 
```

If `dynamic` is used
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants