Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: apiSpec may be const literal #854

Merged
merged 2 commits into from
Jun 2, 2024

Conversation

duncanbeevers
Copy link
Contributor

@duncanbeevers duncanbeevers commented Jun 22, 2023

馃惛 Problem to Solve

The middleware may be instantiated with a string (path-on-disk to schema document) or with an object literal of the schema.

In Typescript, it is possible to import .json files as modules; however the types of their fields are "loose" (eg; the file ['foo', bar'] would be typed as string[]
In order to get stronger typing on imported schemas, they can also be re-emitted as Typescript variables with an as const declaration.

const json = ['foo', 'bar'] as const;

In this case, the json const is typed as its literal value readonly ['foo', 'bar'].
This is great, and can be really helpful both when developing, and at run-time.

However, the express-openapi-validator middleware is unhappy with such as const schemas, since the OpenAPIV3.Document type has many mutable fields.

image

馃憫 Proposed Solution

Through the type system, promise that the express-openapi-validator won't mangle your as const schema.

The middleware is already gentle with the schema, so this is a type-only change.

The change was motivated by the discovery that passing the schema object to other utilities (eg; openapi-typescript) may cause it to be mutated.

The implementation of DeepImmutable is lifted directly from this comment on the TypeScript repo.

describe('default export resolver', () => {
let server = null;
let app = express();

before(async () => {
app.use(
OpenApiValidator.middleware({
apiSpec: {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this json schema is removed... what is the impact of this change on passing a json schema directly in code, similar to what this test previously accepted?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The as const schema is stricter in every way, so existing inline schemas should continue to type-check in the same way.

If a schema is declared as an untyped variable (and not as const) and passed here it may have looser guarantees than if it were declared inline, due to lack of excess property checks.

@cdimascio cdimascio merged commit e35a07c into cdimascio:master Jun 2, 2024
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants