Replace any by unknown in definition files

[ThomasdenH]

Since the new unknown type was added, it makes sense to replace a lot (if not all) usage of any in the definition files with unknown. For example:

Array.isArray(a: unknown): a is unknown[];
JSON.parse(a: string): unknown;

However, this change will cause many errors in projects currently using the fact that the returned types are not type-checked. This means that the change could be controversial.

[RyanCavanaugh]

This is effectively the same proposal as strictAny; see #24737

[ThomasdenH]

If it is true that any has no use over unknown except for compiler error suppression, I might make a script that replaces all usages for my own definition files.

[MadaraUchiha]

This is effectively the same proposal as strictAny; see #24737

This is not exactly correct, the strictAny flag proposal is a project-level compiler flag, whereas this is a proposal to make a change to the definition files to make them more exact.

It makes more sense for JSON.parse() to return an unknown in all cases rather than any in all cases. #24737 makes a case that users need to know unknown better, and I agree, this is a step in that direction.

[MadaraUchiha]

Also, @ThomasdenH, I think you meant

Array.isArray(a: unknown): a is unknown[];

[ThomasdenH]

Yes, you're right!

[ThomasdenH]

I think the main hurdle here is potentially causing confusion when code breaks, but all code should be easy to fix. In the worst case scenario 'as any' will do.

[mohsen1]

For now you can override the types yourself and enjoy more strict type checks:

declare global {
  interface JSON {
    parse(text: string, reviver?: (key: any, value: any) => any): unknown;
  }

  interface ArrayConstructor {
    isArray(a: unknown): a is unknown[];
  }

  interface Body {
    json(): Promise<unknown>;
  }
}

[ExE-Boss]

See also: #27265

Also, Array.isArray(a: unknown) a is unknown[] wouldn’t break the following code or any other code which is using it on a union of types to determine if the argument is an array or not:

function foo(a: string | string[]) {
	if (!Array.isArray(a)) {
		a = [a];
	}
	return a.filter(s => !!s);
}

[ExE-Boss]

@mohsen1 Actually, that should be:

declare global {
	interface JSON {
		/* JSON doesn't support symbol keys, and number keys
		 * are coerced to strings, even in arrays */

		/**
		 * Converts a JavaScript Object Notation (JSON) string into an object.
		 * @param text A valid JSON string.
		 * @param reviver A function that transforms the results. This function is called for each member of the object.
		 * If a member contains nested objects, the nested objects are transformed before the parent object is.
		 */
		parse(text: string, reviver?: (this: unknown, key: string, value: unknown) => unknown): unknown;

		/**
		 * Converts a JavaScript value to a JavaScript Object Notation (JSON) string.
		 * @param value A JavaScript value, usually an object or array, to be converted.
		 * @param replacer A function that transforms the results.
		 * @param space Adds indentation, white space, and line break characters to the return-value JSON text to make it easier to read.
		 */
		stringify(text: unknown, replacer?: (this: unknown, key: string, value: unknown) => unknown, space?: string | number): string;
	}

	interface ArrayConstructor {
		isArray(a: unknown): a is unknown[];
	}

	interface Body {
		json(): Promise<unknown>;
	}
}

[felixfbecker]

This is one of the biggest blind spots regarding type safety in big codebases. You can warn against using explicit any with linting, but you can't change the types of all the declaration files

[aryzing]

This change would be greatly appreciated: I'm trying to check data submitted by users which is supposed to be {foo: string}[] and initially typed as unknown.

As part of the checking process I'm calling Array.isArray, which inconveniently types the data to any[], allowing code such as data[0].bar.baz.qux to be written inside my type guard, which would throw a runtime error.

Currently a dirty workaround I'm using is creating a new var with the appropriate type after the isArray check:

const typedData = data as unknown[];

[evmar]

@RyanCavanaugh now that you've backed off from strictAny, would you reconsider this? The original use case here (which motivated strictAny in the first place) would still be pretty valuable.

[croraf]

Just to note that Body.json() during fetch should also return type Promise<unknown> instead of Promise<any>

[lizraeli]

@RyanCavanaugh I think this is a good proposal, and different from previous ones. One of the places where type safety is most important is when interacting with the outside world - when parsing JSON, making fetch requests, etc. Currently these are all typed as any, effectively allowing us to declare the returned type as anything we want.

To be really type-safe, the return value in these cases should be unknown, and it should be the programmer's responsibility to validate the returned type (e.g. via a custom type guard).

For example, this code is currently valid:

interface CatInfo {
  age: number;
  breed: string;
}

fetch("some-url").then(res => res.json()).then((cat: CatInfo) => {
   // do something assuming the response is a cat
})

But in reality this could easily break. If the type returned by res.json() were Promise<unknown> (rather than Promise<any>), this code would no longer be valid, and we'd need validate the type of the response:

function isCatInfo(value: unknown): value is CatInfo {
   // ...
}

fetch("some-url").then(res => res.json()).then((response) => {
  if (isCatInfo(response)) {
    // the type of response is now CatInfo
  } 
})

I'd love to see a way to at least opt-in to this kind of behavior, so we won't need to override the global type definitions (as in the example by @ExE-Boss) .

[FlorianUekermann]

I'm a bit confused. I don't see any case against unknown being the correct return value in the comments above and this is a pretty obvious source of problems. Given that json is often input of some kind (network, user, files) this issue may even be a major source of security problems (third parties remotely injecting incorrectly typed data via request.json() seems particularly problematic).
Given that this issue has been open for years at this point, could someone give an indication what the problem with the suggested solution is?

[joshuakb2]

I'd like to throw in my support for this proposal. Looks like it hasn't seen much attention lately.

[daniel-white]

Typescript 5 would be a great time to introduce this.

[Samuel-Therrien-Beslogic]

Fans of type-safety here at Beslogic would like this! I've been changing lots of types any for unknown.

[khaosdoctor]

I wanted to express support for @lizraeli's response, since it's been a problem that we will eventually need to resolve in the code. As I was reading up to be able open this issue myself and saw this is already opened (several times #26993, #48697 but with different proposals).

If you're already seasoned as a TS developer, you'll probably know that as X is usually a bad idea in runtime specially because it can introduce erratic behaviour like something coming not the way you expect from outside. So, usually, the default course of action is that we do something with type guards:

type TheType = { required: 'prop' }
function isTypeCorrect (v: unknown): v is TheType { ... }

const value = await (await fetch('something')).json()
if (isTypeCorrect(value)) {
 // do things
}

But, and I've seen this happen some times already (especially with someone that's starting in TS), is that the types are used like:

type TheType = { required: 'prop' }
const value: TheType = await (await fetch('something')).json()

And this is perfectly valid, it can also be true 99% of the times as well, but there's no indication or correct safeguard asserting that it is, which – at least for me – is a bit unsettling, especially if you're working in critical systems.

My initial suggestion before the readup would be to make parse generic with parse<T>, but after #48697 it seems correct to assume that it's even better to just return unknown and make sure that the type guard or the check will be required.

But you can always do as TheType

That's also true, but this would be true for every case, and it's easier to pinpoint in a review when this is done, it's also easier to write an TSLint rule for this as well, but it's not that easy to figure out if the result from fetch is the expected type or if the result is being checked properly.

So I wanted to express the full support for this, especially saying that this is something widely used (we can see numbers in https://github.com/total-typescript/ts-reset where the users alone are 4.1k in GH and 172k downloads/month in npm)

[coolCucumber-cat]

We really need to fix this. If it breaks code, isn't that the entire point of TypeScript? To not allow bad code? And it's not a big deal, because TS doesn't even do anything. Your exising JS will still work if your TS doesn't. We could even go a step further and use generics more and/or better types. For example, JSON.stringify shouldn't allow undefined or BigInts, but you could combine that with generics so that you can have a replacer argument getting rid of those values and then it would be allowed.

[sebastian-fredriksson-bernholtz]

@RyanCavanaugh Is there any update on this? It's been 5 years since this was labeled Awaiting more feedback. No arguments against it seem to have been provided, not even about "breaking change" and "not being able to opt out via a compiler option".

This seems like a major and unnecessary source of unsoundness. Especially changing JSON.parse would force developers to validate data at the edge of the application, or explicitly opt-out of type safety by asserting as any, instead of defaulting to unsound code.

[fabb]

Would be great if this was considered for the next major version update 💞

[alexweej]

I'm actually really bored of teaching less experienced people about this issue. It's a little embarrassing. I realise we have to be careful about breaking source compatibility but the migration of existing code is straightforward — mechanical, even.

[patricio-hondagneu-simplisafe]

I'm surprised this hasn't been added as the default behavior. A compilerOptions entry to disallow it would be great for backwards compatibility though.

[Cafe137]

Obligatory bump for support. 🫡

I strongly believe that typedefs for built-ins should not disable type safety, as it defeats the purpose of the language.

[davidbarratt]

This means that the change could be controversial.

I'm curious where the controversay would be? From what I've seen, people expect the return types to be type checked and are surprised when they learn that they are not.

I actually think this is overall a good thing, it means people implicitly trust TypeScript to keep them safe from type errors. It's a huge miss when that isn't the case.

Obviously this change would "break" a lot of existing projects because TypeScript would throw an error where it previously wouldn't. It therefore seems sensible to have a config option to enable the "strict" behavior.

[MadaraUchiha]

@davidbarratt There are a whooooole lot of silent happy users (who may or may not have bugs around any being in their codebase) and introducing this would necessarily break any code that assumes any at the moment unless they already have casts on every single callsite.

I think everyone are in agreement that a compiler option that will be on by default on "strict": true is the expected behavior here.