Skip to content

Using the "in" operator should make bracket access safe on an object #34867

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

Open
kwasimensah opened this issue Nov 1, 2019 · 10 comments
Open
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@kwasimensah
Copy link

kwasimensah commented Nov 1, 2019

Search Terms

object in keyof bracket operator in

Suggestion

Checking a key with the "in" operator on an object should make it safe to to use in brackets

Use Cases

Useful when writing functions that process input JSON/doesn't have a concrete type.

Examples

//  Ideally this uses TypeScripts control flow logic to allow the bracket access.
function getPropFailsToCompile(data: object, key: string): any {
    if (!(key in data)) {
        throw new Error("Data is malformed")
    }
    return data[key]
}

// Compiles but keyof object has type 'never' in Typescript
function getPropCompilesButNotReasonable(data: object, key: keyof object): any {
    if (!(key in data)) {
        throw new Error("Data is malformed")
    }
    return data[key]
}

// Best way I've gotten this to work.
function getPropWorks(data: object, key: string): any {
    if (!(key in data)) {
        throw new Error("Data is malformed")
    }
    return (<any>data)[key]
}

Checklist

My suggestion meets these guidelines:

  • [x ] This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • [x ] This wouldn't change the runtime behavior of existing JavaScript code
  • [x ] This could be implemented without emitting different JS based on the types of the expressions
  • [x ] This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • [ x] This feature would agree with the rest of TypeScript's Design Goals.
@kwasimensah
Copy link
Author

kwasimensah commented Nov 1, 2019

And just realized 'keyof object' is of type never. So you can't pass anything reasonable to the functions that do compile :)

@jcalz
Copy link
Contributor

jcalz commented Nov 1, 2019

Possible duplicate of #21732

@RyanCavanaugh RyanCavanaugh added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript labels Nov 4, 2019
@RyanCavanaugh
Copy link
Member

This is a bit tricky; we'd need a new kind of type property key that relates to a (hopefully const?) variable, or some special-casing which isn't great. These kind of helper functions are usually short enough and general-purpose enough that having a type assertion or two in the body doesn't change their safety very much, so the trade-off isn't great in terms of complexity. Hearing about more common / safety-requiring scenarios would be good.

@kwasimensah
Copy link
Author

The property accessor can only use string or symbols which are both already immutable in javascript, right?

@RyanCavanaugh
Copy link
Member

The value itself is immutable, but the references to them aren't (unless they're bare const identifiers).

@kwasimensah
Copy link
Author

kwasimensah commented Nov 4, 2019

right, and object isn't immutable either. You'd need something stronger than types to say "This key works for this specific object as long as no else has edited the object or the reference."

The const thing is interesting. I figured your type checks already have something that's smart about reassignments i.e. in the playground this doesn't compile cause it's smart enough to realize that value might be undefined.

function typeErasure(): string | undefined {
    return "hello world"
}

let value = typeErasure()

if (value === undefined) {
    throw new Error("This is undefined")
}

value = typeErasure()

// won't compile cause it's smart enough to realize the 
// reassignment might make this undefined.
let x = value.length

@fatcerberus
Copy link

fatcerberus commented Nov 5, 2019

See #31445 - the compiler would need a way to know that the same key is used for the indexed access as for the in expression, which it currently has no mechanism for; it only looks at the types, and in your case key is an un-narrowed string. I believe this is what @RyanCavanaugh was getting at above.

@nuclearpidgeon
Copy link

nuclearpidgeon commented Dec 11, 2019

The same issue occurs with the for...in loop construct, which may(?) have a slightly narrower solution scope in that it seems the 'key' variable used in the iteration is guaranteed to be a string which can be used to index the object (regardless of the type the property was defined as in the first place), and it ignores the case of Symbol properties.

This seemingly simple example fails compilation at obj[key] unless the function signature is changed to obj: any. (It also fails with signature obj: object).

function processUnknown(obj: unknown) {
    if (typeof obj === 'object') {
        if (obj !== null) {
            for (const key in obj) {
                //     ^^^
                // Typescript identifies key as type `string`

                console.log(`key ${key} indexes ${obj[key]}`);
                //                                ^^^^^^^^
                // Element implicitly has an 'any' type because expression of type 'string' can't be used to index type '{}'.
                //   No index signature with a parameter of type 'string' was found on type '{}'.(7053)
            }
        }
    }
}

However following runtime examples show that this access is safe, regardless of property type. (The only thing I can see breaking this is if somehow the property is removed between const key getting a value and the indexing access op itself.)

processUnknown(JSON.parse('{"someJsonKey":"someJsonValue","someOtherJsonKey":1}'))
// key someJsonKey indexes someJsonValue
// key someOtherJsonKey indexes 1
processUnknown({ "1": "one (string)", 2: "two (number)" })
// key 1 indexes one (string)
// key 2 indexes two (number)

Playground Link for above examples

Similar to the plain in test/expression, there currently doesn't seem to be any way to type-guard down this kind of property access at the moment? Unless you explicitly cast using (obj as { [key: string]: unknown })[key], or rely on variables declarations or function signatures that use { [key: string]: unknown } as a pre-declared type.

Processing parsed JSON shape is the use case I came across this with as well. The example below may motivate things a bit more:

function processParsedJson(id: string, jsonObj: unknown) {
    if (typeof jsonObj === 'object') {
        if (jsonObj !== null) {
            let keysExist = false
            for (const key in jsonObj) {
                keysExist = true
                console.log(`parsed JSON ${id} has key ${key} pointing to ${jsonObj[key]}`);
            }
            if (!keysExist) {
                console.log(`parsed JSON ${id} did not have any keys`)
            }
        }
        else {
            console.log(`parsed JSON ${id} was null object type`)
        }
    }
    else {
        console.log(`parsed JSON ${id} was non-object type ${typeof jsonObj}`)
    }
}

processParsedJson('1', JSON.parse('{"someKey":"someValue","someOtherKey":1}'))
// parsed JSON 1 has key someKey pointing to someValue
// parsed JSON 1 has key someOtherKey pointing to 1
processParsedJson('2', JSON.parse('{}'))
// parsed JSON 2 did not have any keys
processParsedJson('3', JSON.parse('2'))
// parsed JSON 3 was non-object type number
processParsedJson('4', JSON.parse('["a", 2, null, {}]'))
// parsed JSON 4 has key 0 pointing to a
// parsed JSON 4 has key 1 pointing to 2
// parsed JSON 4 has key 2 pointing to null
// parsed JSON 4 has key 3 pointing to [object Object]
processParsedJson('5', JSON.parse('null'))
// parsed JSON 5 was null object type

Playground Link

(Note: JSON.parse currently returns type any, not unknown as this function argument might imply, but hopefully the examples above motivate the kind of type-checking behaviour that you might expect to get if you want to explicitly validate arbitrary JSON shape)

@nightpool
Copy link

nightpool commented Jan 27, 2021

I had the same issue today (Playground Link)

I was able to get around it by defining an existsIn helper as follows:

function existsIn<T>(obj: T, name: string | number | symbol): name is keyof T {
  return name in obj;
}

And then I was able to define the function I wanted

const values = {test: 'this value', foo: 'that value'};

const handleMessage = (name: string): void => {
  if (existsIn(values, name)) { // instead of `name in values`
    console.log(values[name]);
  }
}

maybe there's some unsoundness I haven't considered here, but it seems like it should be pretty simple to have name in obj imply the same name is keyof T assertion that an explicit type guard does!

@stuft2
Copy link

stuft2 commented Nov 18, 2021

@RyanCavanaugh What is the typescript recommended way to determine that an unknown variable is an object with a given property on it?

@nightpool's suggestion is fairly good but like he said, "it should be pretty simple to have name in obj imply the type correctly". I'm always typing something like the following example even thought it doesn't work and I intuitively think it should:

const obj: unknown = { foo: 'bar'}
if (typeof obj === 'object' && obj !== null && Object.hasOwnProperty.call(obj, 'foo') && typeof obj.foo === 'string') {
  //                                                                                             ^^^^^ ERROR
}

A common use case is when catching an error. The error must either be typed as unknown or any but it's a hassle to type check an error that has additional properties on it. For example, node errors usually have a code property on it that can't be type checked without the ritual.

There needs to be a documented, typescript-recommended way of accomplishing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

7 participants