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

LHK-590 Added modern GraphQL Relay sample project with React #420

Merged

Conversation

okhorsunenko-lula
Copy link
Contributor

No description provided.

@xperiandri
Copy link
Collaborator

@andriilula please, review too

@xperiandri
Copy link
Collaborator

@okhorsunenko-lula, @andriilula do we need relay-starter-kit having relay-modern-starter-kit?

@xperiandri
Copy link
Collaborator

xperiandri commented Jan 18, 2023

@okhorsunenko-lula let's format fields input parameter in the same way across the solution FSharp.Data.GraphQL.Relay/Connections.fs
either

fields = [
    Define.Field("hasNextPage", Boolean, "When paginating forwards, are there more items?", fun _ pageInfo -> pageInfo.HasNextPage)
    Define.Field("hasPreviousPage", Boolean, "When paginating backwards, are there more items?", fun _ pageInfo -> pageInfo.HasPreviousPage)
    Define.Field("startCursor", Nullable String, "When paginating backwards, the cursor to continue.", fun _ pageInfo -> pageInfo.StartCursor)
    Define.Field("endCursor", Nullable String, "When paginating forwards, the cursor to continue.", fun _ pageInfo -> pageInfo.EndCursor)
])

or

fields =
    [ Define.Field("hasNextPage", Boolean, "When paginating forwards, are there more items?", fun _ pageInfo -> pageInfo.HasNextPage)
      Define.Field("hasPreviousPage", Boolean, "When paginating backwards, are there more items?", fun _ pageInfo -> pageInfo.HasPreviousPage)
      Define.Field("startCursor", Nullable String, "When paginating backwards, the cursor to continue.", fun _ pageInfo -> pageInfo.StartCursor)
      Define.Field("endCursor", Nullable String, "When paginating forwards, the cursor to continue.", fun _ pageInfo -> pageInfo.EndCursor) ])

What Fantomas will do?

Copy link

@kevin-lula kevin-lula 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 to me

@xperiandri
Copy link
Collaborator

@okhorsunenko-lula so will you replace the old Relay folder content with content from the modern Relay?

@okhorsunenko-lula okhorsunenko-lula marked this pull request as ready for review January 20, 2023 18:42
Copy link
Collaborator

@xperiandri xperiandri left a comment

Choose a reason for hiding this comment

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

@okhorsunenko-lula, looks good!
Fixing a few comments and a workflow update left

samples/relay-modern-starter-kit/src/app.jsx Outdated Show resolved Hide resolved
samples/relay-modern-starter-kit/src/components/user.jsx Outdated Show resolved Hide resolved
samples/relay-modern-starter-kit/src/components/user.jsx Outdated Show resolved Hide resolved
samples/star-wars-api/Schema.fs Outdated Show resolved Hide resolved
samples/star-wars-api/Schema.fs Outdated Show resolved Hide resolved
@okhorsunenko-lula okhorsunenko-lula force-pushed the LHK-590-implement-graphql-relay-cursor branch 2 times, most recently from 68f05dc to b9e3efd Compare January 23, 2023 13:55
.github/workflows/publish_ci.yml Outdated Show resolved Hide resolved
.github/workflows/publish_release.yml Outdated Show resolved Hide resolved
FSharp.Data.GraphQL.sln Outdated Show resolved Hide resolved
@xperiandri xperiandri force-pushed the LHK-590-implement-graphql-relay-cursor branch 2 times, most recently from ea82c58 to 7d7e265 Compare January 24, 2023 05:55
@xperiandri xperiandri force-pushed the LHK-590-implement-graphql-relay-cursor branch from 7d7e265 to 75022b9 Compare January 24, 2023 05:59
@xperiandri
Copy link
Collaborator

@okhorsunenko-lula test Relay again. I made Edges not nullable. I don't understand why the original author made them nullable. Have you seen anything about that in spec?

@xperiandri
Copy link
Collaborator

@okhorsunenko-lula we need to test the sample manually again. Then we can merge

@okhorsunenko-lula
Copy link
Contributor Author

okhorsunenko-lula commented Feb 3, 2023

@xperiandri tested, looks good and works, attaching the screenshot
Can you please merge it?

image

@xperiandri xperiandri force-pushed the LHK-590-implement-graphql-relay-cursor branch from 7daca9f to ca221a8 Compare February 4, 2023 00:53
@xperiandri xperiandri changed the title LHK-590 add relay project LHK-590 Added modern GraphQL Relay sample project with React Feb 4, 2023
@xperiandri xperiandri enabled auto-merge (squash) February 4, 2023 00:57
@xperiandri xperiandri disabled auto-merge February 4, 2023 01:19
@xperiandri xperiandri merged commit fb63c27 into fsprojects:dev Feb 4, 2023
@xperiandri xperiandri deleted the LHK-590-implement-graphql-relay-cursor branch February 4, 2023 01:20
xperiandri pushed a commit to Lula-Archive/FSharp.Data.GraphQL that referenced this pull request Aug 7, 2023
xperiandri pushed a commit to Lula-Archive/FSharp.Data.GraphQL that referenced this pull request Aug 7, 2023
xperiandri pushed a commit that referenced this pull request Aug 10, 2023
xperiandri pushed a commit to Lula-Archive/FSharp.Data.GraphQL that referenced this pull request Aug 10, 2023
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.

3 participants