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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use unknown instead of any for additional properties type #1464

Closed
jmorel opened this issue Jun 18, 2024 · 6 comments · Fixed by #1466
Closed

Use unknown instead of any for additional properties type #1464

jmorel opened this issue Jun 18, 2024 · 6 comments · Fixed by #1466
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@jmorel
Copy link
Contributor

jmorel commented Jun 18, 2024

What are the steps to reproduce this issue?

Create an openapi spec with an object that has additionalProperties set to true:

EmptyObjectWithAdditionalProperties:
  type: object
  additionalProperties: true

What happens?

The generated type will be

export interface EmptyObjectWithAdditionalProperties {
    [key: string]: any;
}

What were you expecting to happen?

I would have liked to have unkown instead of any:

export interface EmptyObjectWithAdditionalProperties {
    [key: string]: unknown;
}

Having any disables all type checks on these values, and since any propagates it can hide errors pretty far in the codebase that could have been caught otherwise.

Having unknown forces the developer to handle all possible type cases. If they decide to then cast the values as any it's then their choice but by default they'll have a strictly typed codebase.

Any logs, error output, etc?

Any other comments?

We could make this configurable of course, but if not I have a small PR ready replacing the hardcoded [key: string]: any with [key: string]: unknown.

What versions are you using?

Operating System:
Package Version: v6.30.2
Browser Version:

@melloware melloware added the enhancement New feature or request label Jun 18, 2024
@ezequiel
Copy link
Contributor

Totally agree with this change. any is too dangerous.

I think this is the correct file: https://github.com/anymaniax/orval/blob/master/packages/core/src/getters/object.ts

Simply replace all instances of [key: string]: any with [key: string]: unknown

@melloware
Copy link
Collaborator

PR is welcome.

@jmorel
Copy link
Contributor Author

jmorel commented Jun 19, 2024

I opened #1466

@melloware melloware added this to the 6.31.0 milestone Jun 19, 2024
@jordan-rigo
Copy link

jordan-rigo commented Aug 9, 2024

Is it configurable? Is there anyway I can override this in a mutator?
I dont like it that I have to cast the type

How do you guys handle this in a nice way?

@ezequiel
Copy link
Contributor

ezequiel commented Aug 9, 2024

Is it configurable? Is there anyway I can override this in a mutator? I dont like it that I have to cast the type

Not configurable. Asserting is a superior alternative because any will mask potential errors.

Previously doing something like this was fine: object.a.b.c, but this was dangerous because none of those properties are typechecked given object.a was type any.

You must narrow the type or assert it. I usually prefer to go with what's simplest given the situation

Further reading:

@jordan-rigo
Copy link

Is it possible to override the input or write a output.override.transformer for example?
I'd like to narrow the unknown type at a higher level. Basically during the model creation, so the correct types are cast and not unkown.

@ezequiel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants