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

Remove nested Argument variable Invariant #1708

Closed
wants to merge 1 commit into from

Conversation

jquense
Copy link
Contributor

@jquense jquense commented Apr 29, 2017

👋 I don't expect to have this merged as is, since it just flat out removes the validation error and doesn't move it. I did think this is probably the clearest way to continue from the twitter conversation yesterday with @KyleAMathews, @wincent and @josephsavona

For some context I have up a POC using RelayModern's compiler to handle query validation and fragment composition in Gatsby gatsbyjs/gatsby#876 We don't use the relay runtime just the tooling to extract and compose queries.

We have this problem where many top-level queries accept complex inputs like

query template_blog_post_Query($slug; String!) {
   markdownRemark(slug: { eq: $slug }) {
      id
      html
    }
}

Normally you could simply work around this by doing slug: $slug but in this case. the actual values of the arguments are provided via a build/tooling step and can't specify the variables alongside the query. In other words $slug is just provided to Pages as variable, and it'd be fairly impractical to provide every possible variant of the inputs.

With regard to this PR i'm happy to move or toggle the validation but I didn't see a good place to do that. so some input is appreciated :)

Thanks ya'll!

PS. While we are at it, it would also be nice to toggle/disable the naming convention requirements for operations if that isn't required by compiler

@josephsavona
Copy link
Contributor

Thanks for creating the PR to start the discussion! The invariant is already in a Relay-runtime specific module: this is the code generator for artifacts used by the runtime. So ideally we'd leave this invariant as is.

Instead we should be able to make it possible to use the compiler without executing Relay codegen, which it sounds like you don't need.

@jquense
Copy link
Contributor Author

jquense commented Apr 29, 2017

Glad to get the ball rolling :) Your comment about bypassing codegen was helpful. I realized I was probably trying to use too much of the compiler code to do this. I've started an exploration of building a simpler Runner with the compiler that skips (as much as possible now) the codegen bits.

I've found tho that I can't actually get the compiler to run or add definitions to it with the public API, without using the IRWriter. In particular it seems that you need to use ASTConvert?

Here are the bits I've got so far. https://github.com/jquense/relay-test/blob/master/compile.js#L96-L100 Still exploring though, as I get a better handle on the codebase.

@jquense
Copy link
Contributor Author

jquense commented Apr 30, 2017

Ok so Yeah after some more playing around It doesn't seem possible to instantiate the Compiler without depending on at least:

RelayCompilerContext
ASTConvert
RelayValidator // sort of

That is provided we add a toggle for disabling codeGen on the compiler. Otherwise if you want to say just get the printed queries, you need RelayPrinter as well.

facebook-github-bot pushed a commit that referenced this pull request May 13, 2017
Summary:
In service of #1708 and complete support of relayIR. I added a test but Jest refused to run on my system, watchman was just staling and eating all my CPU cycles.
Closes #1713

Differential Revision: D5056839

Pulled By: leebyron

fbshipit-source-id: 5a8ebcf60dad59d5a122c85dbe6249c74fb2f72d
@MatthewHerbst
Copy link

We are running into this issue with the following schema code:

// relay-modern query via web client js
profile {
  someCoolField(profiles: [$compare]) @include(if: $fetchComparerProfile) {
    ...
  }
}

Where the profiles argument is defined as:

// schema definition
profiles: {
  type: new GraphQLList(GraphQLString),
},

We get the error: RelayCodeGenerator: Complex argument values (Lists or InputObjects with nested variables) are not supported

Not sure I fully follow the above conversation, but is it related to something that would fix the error we see, or are we just doing something plain wrong?

@leebyron
Copy link
Contributor

leebyron commented Sep 7, 2017

@MatthewHerbst - I think that is just a current limitation of Relay. Instead you could write:

someCoolField(profiles: $compare) and ensure the client provides an array value for the variable.

@jquense - sounds like there's consensus that removing the invariant isn't the right way to carry forward, and @wincent has been doing some exciting work lately that will allow the relay-compiler to be used in broader ways which hopefully will be helpful to you in the future.

I'll close this PR, thanks for the discussion!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants