Errors about openapi spec printed when entering dev shell or running `pnpm generate`

[brprice]

When entering a nix develop shell (or manually using pnpm generate), there are lots of messages printed of the form

Message : Property names must be snake case
Path    : paths,/openapi/sessions/{sessionId}/program,get,responses,200,content,application/json;charset=utf-8,schema,properties,modules,items,properties,types,items,properties,qualifiedModule

It appears that many of these are printed twice, once as a warning and once as an error. All the warnings are in one batch, followed by all the errors.

Note that there are a few different sorts of errors, and many messages are printed. As a simple summary I ran nix28 develop -c 'true' > /tmp/msgs 2>&1 and found 503 lines (though a few are from nix, and not all are warnings etc). The unique messages (ignoring the Path lines) are

Message : A 201 or 202 status code should be returned by a 'create' operation.
Message : All request bodies MUST be structured as an object
Message : API definition should not contain circular references.
Message : Array schemas should define a numeric maxItems field
Message : Array schemas should define a numeric minItems field
Message : Common path parameters should be defined on the path object.
Message : Enum values must be snake case
Message : Major version segment not present in either server URLs or paths
Message : OpenAPI object must have non-empty "tags" array.
Message : OpenAPI "servers" must be present and non-empty array.
Message : Operation "description" must be present and non-empty string.
Message : Operation ids must be snake case
Message : operationIds should follow naming convention: operationId verb should be list
Message : Operation must have non-empty "tags" array.
Message : Operations should not return an array as the top-level structure of a response.
Message : Parameter should have a non-empty description
Message : Path parameter names must be snake case
Message : Path segments must be snake case: copy-session
Message : Property names must be snake case
Message : Query parameter names must be snake case
Message : Request bodies and non-204 responses should define a content object
Message : Schema property should have a non-empty description
Message : Schema should have a non-empty description
Message : Should define a maxLength for a valid string
Message : Should define a minLength for a valid string
Message : Should define a pattern for a valid string

And the worrying lines not of this form are

🍻 Start orval v6.10.2 - A swagger client generator for typescript
'lodash/isPlainObject' is imported by ^@lodash/isPlainObject?commonjs-external, but could not be resolved – treating it as an external dependency
'lodash/isEqual' is imported by ^@lodash/isEqual?commonjs-external, but could not be resolved – treating it as an external dependency
'lodash/uniqWith' is imported by ^@lodash/uniqWith?commonjs-external, but could not be resolved – treating it as an external dependency
'lodash/pickBy' is imported by ^@lodash/pickBy?commonjs-external, but could not be resolved – treating it as an external dependency

and

▲ [WARNING] "import.meta" is not available in the configured target environment ("es2015") and will be empty [empty-import-meta]

    src/primer-api/mutator/use-custom-instance.ts:4:11:
      4 │   baseURL: import.meta.env["VITE_API_URL"],

It is not clear how many of these indicate actual problems (e.g. in our openapi spec). Nor is it clear how many root causes there are. As we understand more it may be sensible to split this issue up. For now, I will use this issue to record my current understanding.

[dhess]

I think there's nothing we can do about the lodash warnings, as those are coming from a dependency.

Regarding the import.meta warning: I think this warning is simply invalid. baseURL is being set correctly both in development (locally) and in production. I've been aware of this one for awhile and I still don't understand why it's complaining.

[brprice]

I understand where the first (in the OP's ordering) batch of messages come from, and have some idea of how "important" they are. These ones are of the form

Message : ...
Path    : ...

Orval seems to call ibm-openapi-validator, which is (surprise!) a validator for openapi specs, written by IBM. By default this validator is stricter than the official spec (it checks against their in-house conventions). In particular, I believe all the snake case errors are not errors per the spec, but only differ from IBM's conventions.

For some more info, see https://github.com/IBM/openapi-validator/issues/173 https://github.com/IBM/openapi-validator/pull/41 https://github.com/IBM/openapi-validator/blob/main/docs/ibm-cloud-rules.md#customization

Note that orval uses ibm-openapi-validator ^0.88.0 (in particular, we currently lock it at 0.88.3: https://github.com/hackworthltd/primer-app/blob/3bb7547f315c402d8a9b13df57ef570fd18f23e4/pnpm-lock.yaml#L14164.

[brprice]

One useful trick is to add "ibm-openapi-validator" : "^0.88.0" to devDependencies in package.json and directly run the underlying linter in verbose mode lint-openapi -v primer-api.json which will then say what rule is being fired.

These rules are defined in https://github.com/IBM/openapi-validator/tree/main/packages/ruleset

[dhess]

pnpm has support for overriding dependencies, as well, so we could also pin it to the latest version that I think you had earlier said might handle these better, or something?

[brprice]

pnpm has support for overriding dependencies, as well, so we could also pin it to the latest version that I think you had earlier said might handle these better, or something?

I think I was confused when I said that. I don't currently think a newer version would make any difference here.

[brprice]

619 will tell orval to not worry about case conventions, dealing with one class of messages. The rest are (along with occurrence counts):

• [ ] 1 Message : A 201 or 202 status code should be returned by a 'create' operation.
• [ ] 1 Message : Major version segment not present in either server URLs or paths
• [ ] 1 Message : OpenAPI object must have non-empty "tags" array. (Despite the use of "must" in the message, the spec does not say it is required)
• [ ] 1 Message : OpenAPI "servers" must be present and non-empty array. (Despite the use of "must" in the message, the spec does not say it is required)
• [ ] 1 Message : operationIds should follow naming convention: operationId verb should be list cf https://github.com/IBM/openapi-validator/blob/8306b49affe53ba5f7676e65504f8512a302479b/packages/ruleset/src/functions/operation-id-naming-convention.js#L81 -- because it is a GET .../sessions (iiuc, a GET which ends in a literal segment), ibm would like the operation id to be listSessions rather than getSessionList.
• [ ] 1 Message : Operations should not return an array as the top-level structure of a response.
• [ ] 2 Message : All request bodies MUST be structured as an object PUT .../{sessionId}/name has a request body of type string, and POST .../copy-session has one of type $ref: UUID, i.e. string. Despite the use of "MUST", the spec explictly has an example of this sort.
• [ ] 2 Message : API definition should not contain circular references.
• [ ] 2 Message : Common path parameters should be defined on the path object.
• [ ] 5 Message : Operation "description" must be present and non-empty string. (Despite the use of "must" in the message, the spec does not say it is required)
• [ ] 5 Message : Parameter should have a non-empty description
• [ ] 7 Message : Array schemas should define a numeric minItems field
• [ ] 8 Message : Operation must have non-empty "tags" array. (Despite the use of "must" in the message, the spec does not say it is required)
• [ ] 8 Message : Should define a maxLength for a valid string
• [ ] 8 Message : Should define a minLength for a valid string
• [ ] 8 Message : Should define a pattern for a valid string
• [ ] 9 Message : Request bodies and non-204 responses should define a content object
• [ ] 11 Message : Array schemas should define a numeric maxItems field
• [ ] 11 Message : Schema should have a non-empty description eg The UUID type/schema has no description. Presumably this will be generated from metadata attached to the backend's UUID type
• [ ] 44 Message : Schema property should have a non-empty description eg PaginatedMeta has a totalItems property. We don't give this a description (presumably in the backend's servant type) -- partly due to laziness on our part, partly due it being somewhat self-explanatory. Some of these probably are less obvious and could do with extra metadata. I guess for hygiene (and to catch new cases) it may be good to add descriptions to everything
• [ ] 65 Message : Enum values must be snake case This is #619

As far as I know, none of these are problems according to the openapi spec, but only for ibm's sense of taste.

[brprice]

There are a few general categories that make up most the warnings here:

• Things should have descriptions We should probably add these
• maximum/minimum bounds should be specified I don't know if we want to add these -- it will make implementation of a conforming service harder, since we will have to ensure that responses are not too long

I suggest we deal with these and then have another think about the miscellaneous other ones.

[dhess]

Many of those sound warnings sound like reasonable changes to make. I suppose we can think of these as linting. But it's not a high priority, in any case.