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

Suggestion: perform excess property checks when spreading an inline object literal #39998

Open
OliverJAsh opened this issue Aug 11, 2020 · 15 comments · May be fixed by #59277
Open

Suggestion: perform excess property checks when spreading an inline object literal #39998

OliverJAsh opened this issue Aug 11, 2020 · 15 comments · May be fixed by #59277
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

@OliverJAsh
Copy link
Contributor

OliverJAsh commented Aug 11, 2020

TypeScript Version: 3.9.2

Search Terms:

Code

declare const someCondition: boolean;

type MyObject = { foo: number; bar?: number };

const a: MyObject = {
  foo: 1,
  bar: 2,
  // ✅ Error because `invalid` is an excess property
  invalid: 3,
};

const b: MyObject = {
  foo: 1,
  ...(someCondition
    ? {
        bar: 2,
        // ❌ `invalid` is an excess property, but we don't get an error here
        invalid: 3,
      }
    : {}),
};

In the example above, I only want to include specific properties when a condition is met. That's the only reason I'm using spread here.

I understand that TypeScript only performs excess property checks inside of object literals. Currently this does not include inline object literals which are being spread inside of another object literal.

In my experience this is a very common code pattern so it would be great if TypeScript handled this.

Expected behavior:

An error

Actual behavior:

No error

Playground Link:

Related Issues:

@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 Aug 20, 2020
@ifeltsweet
Copy link

Absolutely agree with the suggestion here. We do stuff like this all the time:

const company: Partial<Company> = {
  ...(update.name ? { name: update.name } : null),
  ...(update.email ? { email: update.email } : null),
  ...(update.alias ? { aliassssss: update.alias } : null), // no error
};

@OliverJAsh
Copy link
Contributor Author

Just realised, this issue isn't just about excess property checks. We're also missing errors for non-excess properties (#41698):

declare const someCondition: boolean;

type MyObject = { foo: number } & ({ bar: number } | {});

const a: MyObject = {
    foo: 1,
    // ✅ Error
    bar: 'foo',
};

const b: MyObject = {
    foo: 1,
    ...(someCondition
        ? {
              // ❌ Expected error, got none
              bar: 'foo',
          }
        : {}),
};

@maksimf
Copy link

maksimf commented Apr 24, 2021

Is there any update on this? This is really annoying that ts doesn't check that

@dwjohnston
Copy link

Note that this issue isn't just on spreading immediately declared object literals, spreading a function parameter can introduce this issue:

type Bar = {
    a: string; 
};

type Chaz = {
    a: string, 
    c: "zip" | "zap"
}

function usesBar(value: Bar, c: "zip" | "zap") : Chaz {
    return {
        c, 
        ...value, // The excess properties will clobber the c value!
    }; 
}

const d = {
    a: "hello", 
    c: "blah blah blah"
}; 

const e = usesBar(d, "zip"); 
console.log(e);
// {
//  "c": "blah blah blah",
//  "a": "hello"
// }

@gerasimvol
Copy link

same bug

type Params = {
  a?: any;
  b?: any;
}

function func (params: Params) {
  return;
}

const objWithAllowedKeys = { a: 2 }
const objWithForbiddenKeys = { c: 2 }

func({ ...objWithAllowedKeys }) // valid - no error ✅
func({ ...objWithForbiddenKeys }) // valid - error ✅
func({ ...objWithAllowedKeys, ...objWithForbiddenKeys }) // invalid - why no error? ❌

@jamesa3
Copy link

jamesa3 commented May 24, 2022

Any update on this?

Haroenv added a commit to algolia/react-instantsearch that referenced this issue Jun 6, 2022
Similar to #3499, but i did a search for other places props was used literally in -web

We'd get a typescript error for these if microsoft/TypeScript#39998 was fixed, but that doesn't seem to be on their table yet
Haroenv added a commit to algolia/react-instantsearch that referenced this issue Jun 6, 2022
* fix(hooks-web): don't pass widget props to ui components

Similar to #3499, but i did a search for other places props was used literally in -web

We'd get a typescript error for these if microsoft/TypeScript#39998 was fixed, but that doesn't seem to be on their table yet

* Update packages/react-instantsearch-hooks-web/src/widgets/HitsPerPage.tsx

Co-authored-by: François Chalifour <francoischalifour@users.noreply.github.com>

Co-authored-by: François Chalifour <francoischalifour@users.noreply.github.com>
@bdpartridge
Copy link

We've been caught by surprise a few times by this in production; specifically in cases where we renamed an optional property as part of some refactor. To be fair, unit tests could've caught the issues too; we were lacking some coverage. But this behavior can be a little surprising, even for those that have been using TypeScript for a while. Probably because it feels inconsistent with how excess property checking works for object literals in general.

Haroenv added a commit to algolia/instantsearch that referenced this issue Jan 4, 2023
…ct-instantsearch#3501)

* fix(hooks-web): don't pass widget props to ui components

Similar to algolia/react-instantsearch#3499, but i did a search for other places props was used literally in -web

We'd get a typescript error for these if microsoft/TypeScript#39998 was fixed, but that doesn't seem to be on their table yet

* Update packages/react-instantsearch-hooks-web/src/widgets/HitsPerPage.tsx

Co-authored-by: François Chalifour <francoischalifour@users.noreply.github.com>

Co-authored-by: François Chalifour <francoischalifour@users.noreply.github.com>
@RCnowak
Copy link

RCnowak commented Jan 23, 2023

Any update on this? In 2023 we rely too much on a tool which has such a big hole in type security. I can use 'strict' flag but I still loose types due to spreading my objects.

@juanrgm
Copy link

juanrgm commented Feb 24, 2023

Same case here and very surprised 😕.

https://www.typescriptlang.org/play?#code/MYewdgzgLgBCBGArApsKBGAXDA3gWACgZiZkwBDeAG2QBNt4QQbyxCBfGAXl0JNIrU62KACcArsgA0fEgEswAN3JU59GGMlSYAeh2lRokKNnEAdBfxF+JVgE8oACwUBzORGWr1m5Lv1gQAyMTa2J2DkJCUEhYBBQ0ACZuXlCBShpvCWlTGAVPNREs7T0g4xyLMysbWzAHZzA3DxUCjSy-GADSkP5wgk4Icih3ADM5ZAgU-jJ04RhGZmRWDiA

const object1: {
    enabled: boolean
} = {
    enabled: true,
    invalid: true, // error
    ...{
        anythingisvalid: true // no error
    }
}

const object2 = {
    enabled: true,
    invalid: true, // error
    ...{
        anythingisvalid: true // no error
    }
} satisfies {
    enabled: boolean
}

@thepuzzlemaster
Copy link

I came across this unexpectedly as well when trying to conditionally add some properties to a return object.
https://stackoverflow.com/questions/75621849/why-does-using-spread-synax-not-honor-type-safety-for-an-expected-return-ty/75622021#75622021

@Kasik-D
Copy link

Kasik-D commented May 14, 2023

I wrote type to check is T type compare to K type, but I don't know how this code improve and transform to use on function

type Test = { k: string, n : number }

const test = { n: "", k: '' }


type isCurrentObject<T extends object, K extends object> = keyof T[]['length'] extends keyof K[]['length'] ? keyof T extends keyof K ? true : false : false


type a = isCurrentObject<Test, typeof test> 

Playground

@Harpush
Copy link

Harpush commented Jun 7, 2023

A very much needed fix... Spreading objects is a really common case

@devmargooo
Copy link

Same bug even in very simple case

type Foo = {
    foo: string;
}

type Bar = {
    bar: string;
}

const bar:Bar = { bar: "" };
const foo:Foo = { ...bar, foo: "" }; // ❌ invalid but no error 

https://www.typescriptlang.org/play?ssl=10&ssc=37&pln=1&pc=1#code/C4TwDgpgBAYg9nKBeKBvAUFLUBmCBcUAzsAE4CWAdgOYDc6AvuuqJFAEICGpyam2AI26ESFGvSboAxnEokoQ0vi48UqBcKgAiLVAb0Zc4LgLxEaqADprigDQm4hHXtpA

@megaboich
Copy link

I recently encountered this issue and was surprised to find that TypeScript ignores the error. It's astonishing to see that this problem has been around since at least 2017 without being resolved. It's quite disappointing.

@vtgn
Copy link

vtgn commented May 14, 2024

Hi!
I'm totally disappointed too because of this illogical behavior. I've got the problem with a simple case too, similar to the one of @devmargooo above.
It is sad that Typescript can't detect this problem. We have to wait the execution of the software and the class-validator validation of the object to detect the error, whereas TypeScript could have done it as early as during coding in the IDE and during the build. :'(

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

Successfully merging a pull request may close this issue.