search

graphql-rust / graphql-parser

A graphql query language and schema definition language parser and formatter for rust
Apache License 2.0
355 stars 77 forks source link

Why does Document need a lifetime? #41

Closed EverlastingBugstopper closed 4 years ago

EverlastingBugstopper commented 4 years ago

I'm trying to parse a schema into AST, and I'm wondering why the Document type it produces requires a lifetime. I'd like to be able to use the document as an owned struct without having to worry about lifetimes at all.

tailhook commented 4 years ago

We had that in previous implementations, but some users want parser with less memory allocations. So what you can do is parse_query<String>(...)?.into_static() to get a Document with static lifetime (which generally an equivalent of the thing without a lifetime in Rust).

EverlastingBugstopper commented 4 years ago

appreciate it!!

mathstuf commented 3 years ago

So this change is making the update for graphql_client_codegen absolutely ridiculous. I'm hours into chasing down lifetimes and mismatches and I have no idea how deep this rabbit hole will continue to go. Reborrowing in scopes doesn't convince the compiler, there are all kinds of method calls that borrow which require turning into owned data in order to use the types in the future, etc. I'll note that using 'static ends up unifying lifetimes in the wrong places (this is what I'm currently trying to untangle) meaning that other parameters end up being requested to be 'static. Was it really worth removing the non-lifetime variants?

mathstuf commented 3 years ago

Ok, so by just using 'static, String "everywhere" I was able to get to at least update to 0.3. I'll make another attempt to not do so everywhere next.

tailhook commented 3 years ago

I think it's fine to use 'static, String for codegen, as it's not performance critical part of the code.

(I'm sorry that it took you too much time, we probably may improve our docs suggesting that static lifetime is okay?)