Generate schema for isograph.config.json
#292
Conversation
PatrykWalach
changed the title
isograph.config.json
rbalicki2
left a comment
rbalicki2
left a comment
There was a problem hiding this comment.
Looks amazing, love it, just a few nits
| Self::Error | ||
| } | ||
| } | ||
| pub fn generate_json_schema_config() { |
There was a problem hiding this comment.
Can you move this to main.rs? Seems weird to have side effectful functions here
.github/workflows/ci.yml
Outdated
| - name: Build | ||
| run: pnpm -r compile | ||
| - name: Build json schema | ||
| run: pnpm build-json-schema |
There was a problem hiding this comment.
- let's instead have a step where we ensure that the checked in
isograph-config-schema.jsonfile is unchanged after we runbuild-json-schema, and have this run as a dependency ofall-checks-passed
rbalicki2
left a comment
rbalicki2
left a comment
There was a problem hiding this comment.
How are you able to get validation to work with a path that starts with ..? If I copy-paste isograph-config-schema.json into the same folder as the isograph.config.json file, I can get vscode to validate it correctly, but if I have it in ../../whatever, it shows Unable to parse content from '/Users/rbalicki/code/isograph/libs/isograph-compiler/isograph-config-schema.json': Parse error at offset 0.
Do you see this?
.github/workflows/ci.yml
Outdated
| path: artifacts/linux-x64 | ||
| - name: Make artifact executable | ||
| run: chmod +x ./artifacts/linux-x64/isograph_config | ||
| - name: 'Build json schema' |
There was a problem hiding this comment.
nit: you use single quotes a bunch, but I think they aren't really necessary?
.github/workflows/build-cli.yml
Outdated
| if-no-files-found: error | ||
| - uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: ${{ inputs.json-schema-artifact-name }} |
There was a problem hiding this comment.
it looks like we're not using the artifact, except for ubuntu-latest (i.e. as part of build-cli-linux). Can we avoid building and uploading this for the other platforms?
TBH I think it's sufficient to check that the output is the same in ubuntu-latest.
| #[derive(Deserialize, JsonSchema)] | ||
| #[serde(deny_unknown_fields)] | ||
| struct ConfigFile { | ||
| pub struct ConfigFile { |
There was a problem hiding this comment.
this causes title: "ConfigFile" in the schema file, can we change that to IsographProjectConfig or something?
|
I do get |
|
re: vscode not accepting it, sgtm |
| **/.docusaurus/** | ||
| **/dist/** | ||
| **/relay-crates/** | ||
| isograph-config-schema.json |
There was a problem hiding this comment.
Is there a reason we should ignore the json file? It seems like we should always check in the latest version, just like we ensure that the demo artifacts are current (etc)
Wdyt?
There was a problem hiding this comment.
This is prettier ignore, we don't format any of the artifacts.
There was a problem hiding this comment.
So I guess, why are we uploading the artifact (on linux)?
The way I see it, we have the artifact checked in, so instead of uploading and downloading it, we can just use the checked in version?
There was a problem hiding this comment.
To check if it's been built before check-in
- generate a JSON schema for `isograph.config.json`, which users can then refer to via `"$schema": "path to schema"` for some added type safety
2 participants
Supersedes #290