sindresorhus / dot-prop

Get, set, or delete a property from a nested object using a dot path
MIT License
812 stars 69 forks source link

Examples don't seem to have correct typescript types #84

Closed OmgImAlexis closed 2 years ago

OmgImAlexis commented 3 years ago

"Expected" comes from the usage example in the readme and "Actual" comes from typescript's intellisense in VS Code.

The main thing I noticed is the setter and deleter don't seem to mutate the type.

I'm using v6.0.1 of dot-prop with typescript v4.2.4.

import dotProp from 'dot-prop';

{
    // Getter
    dotProp.get({foo: {bar: 'unicorn'}}, 'foo.bar');
    // Expected => 'unicorn'
    // Actual => unknown

    dotProp.get({foo: {bar: 'a'}}, 'foo.notDefined.deep');
    // Expected => undefined
    // Actual => unknown

    dotProp.get({foo: {bar: 'a'}}, 'foo.notDefined.deep', 'default value');
    // Expected => 'default value'
    // Actual => 'default value'

    dotProp.get({foo: {'dot.dot': 'unicorn'}}, 'foo.dot\\.dot');
    // Expected => 'unicorn'
    // Actual => unknown
}

{
    // Setter
    const object = {foo: {bar: 'a'}};
    dotProp.set(object, 'foo.bar', 'b');
    console.log(object);
    // Expected => { foo: { bar: 'b' } }
    // Actual => { foo: { bar: string; }; }

    const foo = dotProp.set({}, 'foo.bar', 'c');
    console.log(foo);
    // Expected =>  {foo: {bar: 'c'}}
    // Actual => const foo: {}

    dotProp.set(object, 'foo.baz', 'x');
    console.log(object);
    // Expected => {foo: {bar: 'b', baz: 'x'}}
    // Actual => const object: { foo: { bar: string; }; }

    // Has
    dotProp.has({foo: {bar: 'unicorn'}}, 'foo.bar');
    // Expected => true
    // Actual => boolean
}

{
    // Deleter
    const object = {foo: {bar: 'a'}};
    dotProp.delete(object, 'foo.bar');
    console.log(object);
    // Expected => {foo: {}}
    // Actual => const object: { foo: { bar: string; }; }

    object.foo.bar = {x: 'y', y: 'x'};
    // Type '{ x: string; y: string; }' is not assignable to type 'string'.ts(2322)

    dotProp.delete(object, 'foo.bar.x');
    console.log(object);
    // Expected => {foo: {bar: {y: 'x'}}}
    // Actual => const object: { foo: { bar: string; }; }
}
mmkal commented 2 years ago

Yeah, @sindresorhus this looks like a bug:

The Get<ObjectType, PathType> extends unknown ? DefaultValue : ... here is wrong - it will always evaluate to DefaultValue because everything extends unknown. It should be changed to unknown extends Get<ObjectType, PathType> ? DefaultValue : ....

There might also be an issue with the tsd tests, they should be catching this. https://github.com/SamVerschueren/tsd/issues/142. I don't know tsd super well but it might be worth switching to expect-type over that bug - it essentially makes every test for this particular library pass no matter what, as far as I can tell.