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

Add resolveQueryFromOperation and JSDoc comments #8

Merged
merged 1 commit into from
Nov 5, 2018

Conversation

javamonn
Copy link
Contributor

@javamonn javamonn commented Nov 1, 2018

  • Adds a resolveQueryFromOperation parameter that is used to resolve the operation query text in cases when operation.text is missing. We have a persisted query implementation in which only a query ID is compiled into the query artifact, and this ID is normally resolved into the full query text on the server. In the case of relay-mock-network-layer however we need a way to perform this resolution client side. See Port Relay test tooling from Reaction, update to Node 10 artsy/emission#1209 (comment) for more information.
  • Adds JSDoc annotations.
  • Runs Prettier over source, I can easily revert this if not desired.

/cc @orta

index.js Outdated
// See https://github.com/facebook/relay/issues/1816
const query =
operation.text ||
(resolveQueryFromOperation && resolveQueryFromOperation(operation));
Copy link
Member

Choose a reason for hiding this comment

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

Should we always call resolveQueryFromOperation if it's passed? It might be useful to transform it in some way unrelated to persisted queries?

Copy link

Choose a reason for hiding this comment

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

I think that's reasonable yeah, could basically just flip the order of this ||

@robrichard
Copy link
Member

@javamonn thanks for the PR. I just added my preferred prettier config to the repo. Can you re-run your changes using this config?

@javamonn
Copy link
Contributor Author

javamonn commented Nov 5, 2018

@robrichard I've updated this in response to your comment and rebased on top of the latest prettier changes.

@robrichard robrichard merged commit 63d5f99 into 1stdibs:master Nov 5, 2018
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