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

context is initially undefined since v1.9.0 #90

Closed
dan-lee opened this issue Nov 3, 2021 · 8 comments
Closed

context is initially undefined since v1.9.0 #90

dan-lee opened this issue Nov 3, 2021 · 8 comments

Comments

@dan-lee
Copy link
Contributor

dan-lee commented Nov 3, 2021

Since v1.9.0 the context in a resolver seems to be initially undefined.

This leads to errors like

Cannot read properties of undefined (reading 'session')

Here's a repro: https://stackblitz.com/edit/node-ysprk8

I've just taken the context example from graphql-helix.
When you switch the versions in the stackblitz from v1.9.0 to v1.8.4 in package.json it will work again.

dan-lee added a commit to launchport/helix-flare that referenced this issue Nov 3, 2021
@hayes
Copy link

hayes commented Nov 3, 2021

this is related to execute removing support for positional args.

Args for execute were converted to be an object, but they key for context should have been updated to be contextValue but is being passed as context

c8750f3#diff-6c8aa63d319597d6d7fff26fda262c760963d5a73d99ad303c0da528bbd376a5R140

Technically this was also a breaking change since any provided context function is now called with completely different arguments.

I think it would probably also make sense to align formatResult with this new API so both are called with contextValue to keep things consistent (this is also a breaking change)

@hayes
Copy link

hayes commented Nov 3, 2021

Happy to submit an PR if you want, just need to know how to resolve this (which combination of the following fixes makes sense).

  1. Just call execute with the contextValue
  2. Update types for custom execute function to accept same arg shape as default execute
  3. Map old execute format to new execute format so API does not change
  4. Update formatPayload to also accept context as contextValue

@dotansimha
Copy link
Collaborator

@ardatan can you please check this?

@ilijaNL
Copy link

ilijaNL commented Nov 4, 2021

Confirmed this. It also breaks usage with getEveloped (https://github.com/dotansimha/envelop)

@n1ru4l
Copy link
Collaborator

n1ru4l commented Nov 4, 2021

Fix is being worked on in #91

@mcrawshaw
Copy link

Confirmed as resolved in 0.9.1.

@bigint
Copy link

bigint commented Nov 4, 2021

Working perfect now thanks!

@n1ru4l
Copy link
Collaborator

n1ru4l commented Nov 4, 2021

🎉

@n1ru4l n1ru4l closed this as completed Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

7 participants