-
Notifications
You must be signed in to change notification settings - Fork 129
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
feat: perform
function that does parsing, validation, context assembly and execution/subscription
#1515
base: main
Are you sure you want to change the base?
Conversation
* experimenting traced schema * Fix * remove comment * do optional chaining since we are not strongly typing * document * cleanup Co-authored-by: Arda TANRIKULU <[email protected]>
…#1506) * remove useAsyncSchema * rename to useSchemaByContext
…1500) * on resolve plugin * changeset * no more onResolverCalled * unused import * args is a record * integrate and useOnResolve * resolversHooksSymbol does not exist * plugincontext for OnPluginInit * on-resolve uses addPlugin * onresolvercalled is no more * refactor for new on-resolve * fix open-telemetry tests * fix newrelic * opentelemetry graphql as peer dep * tests * addPlugin doesnt need to be used * reorder * respects onPluginInit context * drop unused import * fixes false positive eslint warnings Co-authored-by: Dimitri POSTOLOV <[email protected]>
* feat: trigger on context, validate and parse errors * trying * make it work * pass in phase * add phase details * feedback
This reverts commit 9a59c62.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is still no way for applying the onPerformDone
logic when not using the perform
function. Which makes it impossible for users that do not use perform
to leverage plugins that use the onPerformDone
hook 🤔 .
I think we should provide such an path as well.
I added the
@n1ru4l that is the thing, if you don't use Perform is the only function in envelop that wraps parsing and validation errors - you must use it to leverage plugins using these hooks. The problem is described in #1493. Do you have some alternatives maybe? |
* docs: migration guide * remove slashes * Update website/docs/guides/migrating-from-v2-to-v3.mdx Co-authored-by: Denis Badurina <[email protected]> * feedback * document removing of orchestrated tracer * more feedback * update docs * update examples * async schema example Co-authored-by: Denis Badurina <[email protected]>
If I understand well what n1ru4l says, it's not a matter of using or not |
Yes, I understand this too. But I don't see a lot of feasible solutions, maybe one of them would be: log a warning (throw error? too harsh?) if a plugin uses the perform hooks but the invoker didn't use perform. 🤔 |
What if we augment each gql function so that it calls
This way, users can notice when their results are incomplete with respectful plugins and read about how to fix it - the plugins should state this. However, if the plugin doesn't expect changing full exec results for parsing and validating - nobody will notice. Still there is this question: how to invoke |
a23dec1
to
3ac13be
Compare
Closes #1493
Necessary for #1490
perform
function usageonPerform
andonPerformDone
plugin usageTODO