Fix lint and add lint + prettier to CI #461
Conversation
| @@ -4,7 +4,7 @@ module.exports = { | |||
| es2021: true, | |||
| }, | |||
| root: true, | |||
| extends: ['standard-with-typescript', 'plugin:react/recommended'], | |||
There was a problem hiding this comment.
This plugin only appears to be used in app, so configuring it here seems to be a mistake
There was a problem hiding this comment.
It was intentional I think, because it didn't harm anything, but can't remember much beyond that
| @@ -211,7 +211,9 @@ export class HttpCallNodeImpl extends NodeImpl<HttpCallNode> { | |||
| const method = getInputOrData(this.data, inputs, 'method', 'string'); | |||
| const url = getInputOrData(this.data, inputs, 'url', 'string'); | |||
|
|
|||
| // TODO: Use URL.canParse when we drop support for Node 18 | |||
There was a problem hiding this comment.
Not sure if we have an official Node support policy
| @@ -74,6 +74,7 @@ export type ChatCompletionChunk = { | |||
| model: string; | |||
| }; | |||
|
|
|||
| /* eslint-disable @typescript-eslint/naming-convention */ | |||
There was a problem hiding this comment.
Not sure if it is better to leave this to match API naming convention or rename to match Rivet/JS naming convention
|
The new checks thingy is pretty nice although it does mean the existing warnings will appear on every PR until they are resolved (but it also means new warnings will show up more prominently which is nice) |
|
I'll merge this for now so we can also run lint on the new PRs coming in, the warnings should be fixed later |
I've noticed while there is a lint and prettier command configured, they are not actually enforced by CI, so some of the code has drifted. Most can be automatically or manually fixed but a few places where the linter has picked up genuine bugs has been left due to lack of context needed to fix them.
This PR tries to bring the code up to lint as much as possible conservatively and add them to CI so they don't drift in the future.
The one big bug that it found is an import cycle between RunGraphNode and the globalRivetNodeRegistry global. There's no easy way to reconcile this other than making the global registry lazy by putting it behind a getter, but that's more code alteration than seems good for a lint fix PR so I'll delegate that to a future PR to fix.