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

GraphQL example is missing labels #577

Closed
wtrocki opened this issue Aug 13, 2019 · 9 comments
Closed

GraphQL example is missing labels #577

wtrocki opened this issue Aug 13, 2019 · 9 comments
Assignees
Labels
Type: Feature New features and feature requests

Comments

@wtrocki
Copy link

wtrocki commented Aug 13, 2019

When creating GraphQL example labels will not be provided.
After checking source code I found that extra object needs to be passed.
For example:

    city:   {label: 'City'},
    state:  {label: 'State'},
    street: {label: 'Street'},
    zip:    {label: 'Zip'}

Can labels be derived from ast values?
Example: https://astexplorer.net/#/gist/aaf298cda868080a86c1e1838c9fb382/a98e0667d362bdba99fafeee99818b8c411c05be

@wtrocki
Copy link
Author

wtrocki commented Aug 13, 2019

Workaround for the moment is to use something like this:

visit(parse(schema), {
    FieldDefinition(node) {
        schemaData[node.name.value] = {
            label: node.name.value
        };
    }
})

@radekmie radekmie self-assigned this Aug 14, 2019
@radekmie radekmie added the Type: Feature New features and feature requests label Aug 14, 2019
@radekmie
Copy link
Contributor

Hi @wtrocki. It's something I thought about it for a long time. It's not the best solution, as the schema names often are not very user-friendly, but they can definitely be a sensible default. It'll also unify DX between bridges.

@wtrocki
Copy link
Author

wtrocki commented Aug 14, 2019

Hmm.. I do not need to be in the core, but we can provide helper to wrap functions as above. Community contribution?

@radekmie
Copy link
Contributor

Well, yeah, but it really makes sense to put it in the core. GraphQLBridge#getProps to be exact.

@wtrocki
Copy link
Author

wtrocki commented Aug 17, 2019

I have put some thought into entire graphql implementation and I think it needs some work to make it useful for production cases.

Main pain points I see now is:

Performance

Generally parsing GraphQL schema (as oposed to documents) is quite slow.
Getting thru complex AST was giving me extra 62 ms webpage rendering time on browser and 151ms on random phone. Using json schema do not add such overhead. The fact that this is done for every client refresh ask for improvements.

GraphQL specific issues

Wrapping schema into form seems easy in hello world scenario but for normal use cases schema will diverge from form. Example? ID field in the schema is very rarely used as ID is assigned by server?

Generally idea of GraphQL is that there is flexibility around what is sent to server and what is returned. Basing on type definitions is going to work with limited use cases. To be clear uniforms should be basing on InputTypes passed to mutations to be more aligned with entire concept

How to address this issues

Generally GraphQL integration seems like just starting. I think the best GraphQL intergartion is no integration at all.

Imagine that I will put very complex schema into some cmd on build time and generate json schema. Then I can extend/modify json schema, push it to repo etc. without performance or usability penality. We can add extra directives to generator engine that can cover validation use cases etc.

What this means is that uniforms will still be supporting graphql but in more usable way.

I'm working now on integrating uniforms with http://graphback.dev and I decided to not use GraphQL bridge

@wtrocki
Copy link
Author

wtrocki commented Aug 17, 2019

My assumption is that Uniforms GraphQL implementation allows people to use schema they have on server. Correct me if I'm wrong

@radekmie
Copy link
Contributor

radekmie commented Aug 17, 2019

Generally parsing GraphQL schema (as oposed to documents) is quite slow. [...]

Agree. It's up to you if you do it on runtime: it's possible to do it during the build. gajus/babel-plugin-graphql-tag is one option.

[...] ID field in the schema is very rarely used as ID is assigned by server?

There's the AutoFields component with omitFields prop for that. That's also why we don't provide any default validator and let users do it.

[...] To be clear uniforms should be basing on InputTypes passed to mutations to be more aligned with entire concept

It's not stated in the docs, but it also works with input types. Check out the GraphQLBridge tests.

Generally GraphQL integration seems like just starting. I think the best GraphQL intergartion is no integration at all. [...] I'm working now on integrating uniforms with http://graphback.dev and I decided to not use GraphQL bridge

I agree that no default nor go-to validator for GraphQL forces us to have another one. JSON Schema is a great choice here - we do mix them internally too.

But on the other hand, there are use cases for the current integration. You can create a form prototype instantly for any of your (input) types. In some situations, it's easier (and better in terms of bundle size) to use it as it is and write the validator by hand. The types are already there and so is the parsed schema (again, you can do it on a build step), so using it to make a form is often a no-brainer.

We can add extra directives to generator engine that can cover validation use cases etc.

That's something that IMO decides on the schema choice. It'll be fairly easy (in terms of integration, possibly not implementation) to do it using ajv keywords or similar extensions.

My assumption is that Uniforms GraphQL implementation allows people to use schema they have on server. Correct me if I'm wrong

Of course, it does! It also allows you to create a full-blown app with it. Maybe it's not as obvious as with JSON Schema, but it's doable (and people do it).

@wtrocki
Copy link
Author

wtrocki commented Aug 17, 2019

Actually this gives me more perspective and info to work on. I will give second go to GraphQL bridge and see if I can get something like @OmitFieldDirective

@wtrocki
Copy link
Author

wtrocki commented Aug 19, 2019

I'm playing with this and getting some strange results when using input types.
I will follow up to see if that is issue on my side - if not going to create new issue:

Screenshot 2019-08-19 at 14 50 37

Ignore me for now and btw. good work on autolabels!
And yes problem was on my side!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New features and feature requests
Projects
Archived in project
Development

No branches or pull requests

2 participants