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

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

Closed
OmgImAlexis opened this issue May 2, 2021 · 1 comment · Fixed by #92
Closed

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

OmgImAlexis opened this issue May 2, 2021 · 1 comment · Fixed by #92

Comments

@OmgImAlexis
Copy link

"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
Copy link
Contributor

mmkal commented Jan 29, 2022

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. tsdjs/tsd#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.

mmkal added a commit to mmkal/dot-prop that referenced this issue Jan 29, 2022
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 a pull request may close this issue.

2 participants