-
Notifications
You must be signed in to change notification settings - Fork 470
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
[language server] Allow configuration of validation rules. #1288
[language server] Allow configuration of validation rules. #1288
Conversation
3a46d45
to
1e5094b
Compare
94ece0b
to
328c206
Compare
I've seen that CI failure before and it went away after a re-run by fixing a spelling error in a comment and force pushing. I can do the same again, but perhaps it’s better to highlight this gremlin? |
Oops, forgot to mention you before, @JakeDawkins. |
328c206
to
f67e0d6
Compare
I added the ability to specify a function that can be used to filter the default validation rules. The reason for this is that when using the extension on non-javascript projects, there is no good way to import the default validation rules from the Apollo package without adding a In our case, we have Ruby applications that we want to use the extension on, but we want to filter out the |
@alloy this is fantastic, thank you! I'm going to give this a final pass closer to EOD, then we'll merge and cut a release. At first glance, I've got no changes to suggest 😄 |
This LGTM. There are a couple tasks I'd like to spin out of this.
If you're up for both or either, just let me know - otherwise I'll take care of them. Thanks again! |
@alloy FYI, I cut a release but can't get the extension working (unrelated to your changes) so I'll need to get a fix together before I can get that packaged up and posted on the marketplace. Sorry about the delay! I'm aiming for tomorrow, but I'll keep you looped in either way. |
For context on the issue, see: #1306 Thanks again! |
Awesome, thanks!! 🙏 Yeah I can do those other tasks 👍 |
Summary: Closes #2518 _Note: Includes #2745, so that should probably be merged first and then this can be rebased._ I tried multiple times to come up with a doc describing the integration a few times, but then yesterday I had a whole day and figured I’d just write _a_ implementation to show what it could look like. Please feel free to poke holes as much as you need. ---- What this change does: * Add a new `relay-config` package * Usage of the package is optional. If installed it will be picked-up, otherwise relay will function as it does today. * RelayConfig relies on [cosmiconfig](https://github.com/davidtheclark/cosmiconfig) to do its bidding. It’s configured to load from: - a `relay` key in `package.json` ```json { "relay": { "src": "./src" } } ``` - a `relay.config.json` file ```json { "src": "./src" } ``` - or a `relay.config.js` file ```js module.exports = { src: "./src", } ``` * It accepts all the same configuration as the CLI does. (As a bonus, I typed the configuration object and the Yargs options object.) * Additionally, when using the `relay.config.js` file, a configuration entry like the language plugin also accepts an actual function: ```js const typescript = require("relay-compiler-language-typescript") module.exports = { language: typescript, } ``` In the future, other entries such as `persistedQueries` and `customScalars` could also be configured as such and allow for project specific setup. * It is used by all pieces of Relay that take configuration–i.e. `relay-compiler` and `babel-plugin-relay`. For instance, previously one would have needed to specify `artifactDirectory` on the CLI to `relay-compiler` and the same setting in `.babelrc` for `babel-plugin-relay`. This can now be reduced to a single centralised setting. Additional external tooling can also start relying on this. For instance, the `vscode-apollo` extension, with [this change](apollographql/apollo-tooling#1288), will allow for custom GraphQL validation rules to surface in the UI. Leveraging the centralised config allows one to surface `relay-compiler` specific validations without needing to take in the same configuration in a different place yet again. * Finally, this also adds some previously missing test coverage around language plugin loading. Pull Request resolved: #2746 Reviewed By: josephsavona Differential Revision: D15721312 Pulled By: jstejada fbshipit-source-id: d9b76ac1866a9b0f2c2a76f65a16f79a28d4bc24
Closes #1056
(I didn’t find any test coverage for the areas I touched, but am pretty sure my additions are backwards compatible.)
Before
Config
After