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

fix: improve setSchema & schema loading, allow primitive schema #1648

Merged
merged 3 commits into from Aug 22, 2020

Conversation

cshaver
Copy link
Collaborator

@cshaver cshaver commented Aug 18, 2020

Fixes #1621

@cshaver
Copy link
Collaborator Author

cshaver commented Aug 18, 2020

whoops, this is creating multiple worker threads in the example 😅
image

@acao
Copy link
Member

acao commented Aug 18, 2020

@cshaver excellent approach! and re: multiple workers that is expected! there should be a graphql, json, and editor language worker. not sure what the fourth is though?

either way... i don't think it's creating multiple workers? i'll pull the repo and see for myself

again, thanks for this, i was gonna try to get to this tonight before vacation but this is perfect timing!

@acao
Copy link
Member

acao commented Aug 21, 2020

@cshaver can you do quick format fix or shall I? we delayed the camping trip til tomorrow so I'm gonna try to get a few releases out today!

@acao
Copy link
Member

acao commented Aug 21, 2020

for whatever reason, if i remove setSchemaConfig() so that only setSchema() is used, it seems the LSP service is being re-instantiated. not sure if that's caused by this PR so i'll have to compare (probably not)

image

I'd like to close this PR once we can guarantee setSchema works on it's own. thinking i can add a toggle to the example app for "local vs remote" schema options

@acao
Copy link
Member

acao commented Aug 22, 2020

what is ultimately happening here
image

I got it working!

I also have GraphQLSchema primitives working for setSchema()! and i think i simplified things in the process.

Now, the API instance will hold onto the static schema if present as worker instances are re-created!

- `api.setSchema()` allows primitive `GraphQLSchema`
- `api.setSchema()` works standalone, without using  `setSchemaConfig()`
@acao acao changed the title [monaco-graphql] fix: TypeError when calling setSchema before worker is loaded fix: improve setSchema & schema loading, allow primitive schema Aug 22, 2020
@acao acao merged commit 975f29e into graphql:main Aug 22, 2020
thenamankumar pushed a commit to thenamankumar/graphiql that referenced this pull request Aug 30, 2020
…hql#1648)

- allow null schema
- `api.setSchema()` works on it's own, and static string is evaluated as a schema
- `api.setSchema()` now allows `string | GraphQLSchema`

Co-authored-by: Rikki <rikki.schulte@gmail.com>
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.

monaco-graphql: GraphQLAPI.setSchema results in TypeError
2 participants