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

Dependencies to query file and schema file are not tracked by the compiler when deriving GraphQLQuery #215

Open
upsuper opened this issue Jan 12, 2019 · 8 comments

Comments

@upsuper
Copy link
Contributor

upsuper commented Jan 12, 2019

When using #[derive(GraphQLQuery)], after compiling, if you change the query file or the schema file and run cargo build again, it would do nothing.

This is because compiler is unaware of the change to the file.

To fix this, the macro should insert include_str! or include_bytes! into the generated code, so that compiler can be aware of such build dependency.

See pest-parser/pest#272 as an example of this trick.

@theduke
Copy link
Member

theduke commented Jan 15, 2019

Haha @upsuper I just wanted to create an issue for this, also suggesting the pest workaround.

( I remember the pest docs instructing you to add the include_str! manually in the past).

@tomhoule
Copy link
Member

Thanks for the trick! We should definitely do this. There could be a compile time impact with huge schemas, but let's see.

@tomhoule
Copy link
Member

One potential issue is that we work with file paths relative to the Cargo.toml, but include_str! works with paths relative to the current file, I think, so we need to map them somehow.

@upsuper
Copy link
Contributor Author

upsuper commented Jan 15, 2019

You can just use an absolute path.

tomhoule added a commit that referenced this issue Feb 12, 2019
This will force the regenaration the generated modules when query files
change.

See issue 215: #215

Also extract GraphQLClientCodegenOptions to its own module, and use
accessors instead of a raw data struct.
@tomhoule
Copy link
Member

PR #223 addresses this - it doesn't include the schema however, as I'm afraid of the performance impact of including a large schema in all generated modules. It may be irrational though.

tomhoule added a commit that referenced this issue Feb 12, 2019
This will force the regenaration the generated modules when query files
change.

See issue 215: #215

Also extract GraphQLClientCodegenOptions to its own module, and use
accessors instead of a raw data struct.
tomhoule added a commit that referenced this issue Feb 12, 2019
This will force the regenaration the generated modules when query files
change.

See issue 215: #215

Also extract GraphQLClientCodegenOptions to its own module, and use
accessors instead of a raw data struct.
tomhoule added a commit that referenced this issue Feb 12, 2019
This will force the regenaration the generated modules when query files
change.

See issue 215: #215

Also extract GraphQLClientCodegenOptions to its own module, and use
accessors instead of a raw data struct.
tomhoule added a commit that referenced this issue Feb 12, 2019
This will force the regenaration the generated modules when query files
change.

See issue 215: #215

Also extract GraphQLClientCodegenOptions to its own module, and use
accessors instead of a raw data struct.
tomhoule added a commit that referenced this issue Feb 12, 2019
This will force the regenaration the generated modules when query files
change.

See issue 215: #215

Also extract GraphQLClientCodegenOptions to its own module, and use
accessors instead of a raw data struct.
@tomhoule
Copy link
Member

This was just released as part of 0.7.0. Let's see if including the schema file in generated code will turn out to be necessary as well.

@tomhoule tomhoule reopened this Mar 21, 2019
@mathstuf
Copy link
Contributor

See also #117 which resolved to just mention this limitation in the readme.

@tomhoule
Copy link
Member

tomhoule commented Mar 21, 2019

Thanks for the heads up, we should mention this in the README indeed.

edit: I misinterpreted your message apparently, sorry. Anyway, we should track the cargo-based solution because I feel uneasy about the situation with schema files, as mentioned in this thread.

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

No branches or pull requests

4 participants