-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[epic] enable Rust compiler for OSS #3180
Comments
Summary: See #3181 and #3180. This PR adds a language option to the typegen_config and refactors the relay-typegen crate to allow completely different output based on the configured language, currently typescript and flow (which is the default when no language is configured, or an invalid one is set). I also made some changes so the watchman query looks for .ts files when using typescript, and the output files also use .ts as their extension. This probably needs a bit more work before it's completely ready. Off the top of my head: - [ ] In `relay-compiler/build_project/artifact_content.rs` some output is generated such as `flow` and a commented out `import type` statement? Those should probably be left out for Typescript. - [x] Currently the language used for typegen is Typescript if the language in the config file is "typescript", and Flow otherwise. We should probably display an error if an invalid language is set, e.g. "typesrcipt". - [ ] I was not able to actually test the compiler due to an error returned by Watchman: `failed to parse query: must use ["suffix", "suffixstring"]`, and I don't have a clue why. This error was already returned before I made the changes to look for .ts files when using Typescript. - [ ] I did not at all look into `relay-lsp` and whether any changes are needed there for Typescript. Looking forward to your feedback! cc kassens josephsavona maraisr Pull Request resolved: #3182 Reviewed By: tyao1 Differential Revision: D23670620 Pulled By: kassens fbshipit-source-id: 0e3e9e6860c6701d934b406c895dc04bfb5b6322
@MaartenStaa made a huge leap start on this with #3182! I understand there's a few improvements we can make to the types still by copying more what the JS OSS typescript generator produces: https://github.com/relay-tools/relay-compiler-language-typescript/blob/e1b092f081d4e816ecd17fb0d3b4e62cdf3db917/test/__snapshots__/TypeScriptGenerator-test.ts.snap I'll file separate issues for the remaining checkboxes. |
A good intro way to contribute is #3199 to convert the runtime unit tests to use artifacts generated by the Rust compiler. More details in there. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Thanks stale-bot. I think we still need the issue. I'm looking at converting the rest of the tests. It may take a while, but we'll see. |
One more point on why we may need to work on this: the current watchman client for rust doesn't work on Windows (I was testing it with github actions for windows: https://github.com/alunyov/relay/runs/2104656440?check_suite_focus=true). So we may add something like: https://github.com/notify-rs/notify in OSS. |
Baby steps, the example project todo app now compiles with the rust based compiler: relayjs/relay-examples@470c497 NOTE: this is currently linux + mac only and requires watchman. Windows support is definitely a hard blocker for us, this is just an early preview to see where the problems are. |
Can we also look at supporting "watch-mode" for OSS? Currently we're forced to use
|
Will rust compiler support es modules? Looks like it was not implemented. Would be good to have it from the beginning to avoid shipping separate features without support like happened with refetchables. |
@TrySound do you actually know where that Or is that more about |
Oh, yes, this is something that we need to fix. |
Friendly tips: If you have plans to expose |
Hi, I would like to propose to include the following issues/bugs into the list as well:
Thanks! 😎 |
|
There is a problem with
Query: fragment noteOwnerUsers on Query
@argumentDefinitions(first: { type: "Int!" }, after: { type: "String" })
@refetchable(queryName: "noteOwnersUsersConnection") {
users(first: $first, after: $after)
@connection(key: "noteOwnersUsersConnection_users") {
edges {
node {
id
email
}
}
}
}
|
@huv1k this is actually a proper validation that has started working with the new compiler. So, the solution is to go ahead and remove the default value, or make the argument optional by removing the |
v13 released. Closing this in favor of #3749 |
This issue collects what I think is missing to update the Relay compiler to the Rust version in OSS. If someone want's to give any of this a shot, please comment here!
config.json
into a watchman query for watch mode and simple builds. This was designed with non-watchman in mind and it should be possible to build out something that uses a different file-crawler and maybe file watcher crate. This is not strictly required, but I think removing this external required dependency would be nice. (Details in [rust] add non-Watchman file source #3193)flow-bin
package might be a good inspiration as that somehow loads the binary for that version on first use. (Details/discussion in [rust] figure out how to deploy pre-built binary to npm #3194)The text was updated successfully, but these errors were encountered: