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

Error on types when using multiple conditional properties #58432

Closed
Arkellys opened this issue May 4, 2024 · 4 comments
Closed

Error on types when using multiple conditional properties #58432

Arkellys opened this issue May 4, 2024 · 4 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@Arkellys
Copy link

Arkellys commented May 4, 2024

🔎 Search Terms

"jsdoc conditional properties", "typescript conditonnal properties", "type conditon"

🕗 Version & Regression Information

This is the behavior in every version I tried, and I reviewed the FAQ for entries about conditional types

⏯ Playground Link

https://www.typescriptlang.org/play/?ts=5.4.5#code/FASwdgLgpgTgZgQwMZQAQGED2YAmIIjboBqCANgGLkDOaA3sKk6kgPwBcqiZtwAvsFCRYiFBmx4CRUmQAqMAK71GzJJwiKo-QeGjxkaLLnyEwAIRlUey5qgBGHLjS0Che0YYknsF8vKWoDLZ26prabiIGqGYItIEqTAic1BrgAOb8ANyCSNgpqND5ALyoABS5YHAgaZwxcQBkZUaSpr5ymqgAPuLGUuaWzgCUqI2lzd5gJH4d3eN9U5RDw0UAfE7WwBX5MFAQCjDmmJhkqCWlAG7knHZHZFAIYMPXt-dgp2uol2TZwIUQpXRUElUAAiBAggA09k4Oz2BzMt1KGiUwz4gyAA

💻 Code

interface ConditionCValFalse {
    c?: false
}

interface ConditionCValTrue {
    c: true
}

interface ConditionBValFalse {
    b?: false
}

interface ConditionBValTrue {
    b: true
}

interface Base {
    a: string
};

const test = (config: Base & (ConditionBValTrue | ConditionBValFalse) & (ConditionCValTrue | ConditionCValFalse )) => false
const returnBool = (val: boolean) : boolean =>  val;

test({ a: "a", b: returnBool(true) }) // Type 'boolean' is not assignable to type 'true'

🙁 Actual behavior

When using a second set of conditional properties, the first one throws a type error while it passes when using the condition alone.

Argument of type '{ a: string; b: boolean; }' is not assignable to parameter of type 'Base & (ConditionBValFalse | ConditionBValTrue) & (ConditionCValFalse | ConditionCValTrue)'.
  Type '{ a: string; b: boolean; }' is not assignable to type 'Base & ConditionBValTrue & ConditionCValTrue'.
    Type '{ a: string; b: boolean; }' is not assignable to type 'ConditionBValTrue'.
      Types of property 'b' are incompatible.
        Type 'boolean' is not assignable to type 'true'.(2345)

If I remove the second condition, or that I passes true or false directly, I get not error:

const test = (config: Base & (ConditionBValTrue | ConditionBValFalse)) => false
const returnBool = (val: boolean) : boolean =>  val;

test({ a: "a", b: returnBool(true) })
const test = (config: Base & (ConditionBValTrue | ConditionBValFalse) & (ConditionCValTrue | ConditionCValFalse)) => false
const returnBool = (val: boolean) : boolean =>  val;

test({ a: "a", b: true })

Somehow, when passing the c property – which is not supposed to be required – it passes:

const test = (config: Base & (ConditionBValTrue | ConditionBValFalse) & (ConditionCValTrue | ConditionCValFalse)) => false
const returnBool = (val: boolean) : boolean =>  val;

test({ a: "a", b: returnBool(true), c: true })

It also passes if I only declare the a required property:

test({ a: "a" })

So b and c are indeed optional.

🙂 Expected behavior

I expect the type checking to be the same whether I passes one or multiple conditional properties. I believe all of these should work:

// Pass
test({ a: "a" })
test({ a: "a", b: true })
test({ a: "a", c: false })
test({ a: "a", b: returnBool(true), c: returnBool(false) })
test({ a: "a", b: returnBool(true), c: false })
test({ a: "a", b: true, c: returnBool(false) })

// Currently fail
test({ a: "a", c: returnBool(true) })
test({ a: "a", b: returnBool(true) })

Additional information about the issue

For the context, I'm trying to make some properties required based on other property values.

I realize that maybe there is better way to do this in TS, but actually my problem is in JSDoc, and I believe I'm more limited on what I can do with JSDoc synthax.

/**
 * @typedef {object} ConditionCValFalse
 * @property {false} [c]
 */

/**
 * @typedef {object} ConditionCValTrue
 * @property {true} c
 */

/**
 * @typedef {object} ConditionBValFalse
 * @property {false} [b]
 */

/**
 * @typedef {object} ConditionBValTrue 
 * @property {true} b
 */

/**
 * @typedef {object} Base
 * @property {string} a
 */

/**
 * @param {Base & (ConditionBValTrue | ConditionBValFalse) & (ConditionCValTrue | ConditionCValFalse)} config
 */
const test = (config) => false;

/**
 * @param {boolean} val
 * @returns {boolean}
 */
const returnBool = (val) =>  val;

test({ a: "a", b: returnBool(true) }); // Type 'boolean' is not assignable to type 'true'

My real code is on a React component, and unlike the example above, even passing all the optional properties throws the error.

@jcalz
Copy link
Contributor

jcalz commented May 4, 2024

The fact that this works sometimes is due to #30779, which is limited no matter what. It looks like if the discriminant is optional and the property is missing, #30779 doesn't kick in, and you get the same error you'd get when trying to equate a general union-of-objects with an object-of-unions. I don't know if they'll consider this a bug or just a limitation of the sort of thing handled by #30779.

@Arkellys Arkellys changed the title Error on types when using multiple conditionnal properties Error on types when using multiple conditional properties May 4, 2024
@RyanCavanaugh
Copy link
Member

Yeah, in general a single source type S isn't assignable to a target type T = T1 | T2 | T3 | Tn... just because a combinatorial explosion of S would ultimately result in every possible inhabitant eventually finding varying targets in T. There are exceptions, but this isn't one of them.

There's almost always a better way to write the target type; I'm not really clear on the intended semantics of the type as written here so can't advise much

@RyanCavanaugh RyanCavanaugh added the Design Limitation Constraints of the existing architecture prevent this from being fixed label May 6, 2024
@Arkellys
Copy link
Author

Arkellys commented May 7, 2024

If it helps to give a more concrete example, I need this for a React component with several props required or unnecessary depending on other props. For example:

<Chip
  removable={isRemovable} // Optional
  onRemove={noop} // Required if `removable` is `true`, unnecessary if removable is `false`
  label="test" // Required when no `children`, unnecessary when `children` is defined
>
  Test {/* Required when `label` is not defined */}
</Chip>
/**
 * @typedef {object} ChildlessProps
 * @property {string} label
 */

/**
 * @typedef {object} ParentProps
 * @property {any} children
 */

/**
 * @typedef {object} RemovableProps
 * @property {true} removable
 * @property {Function} onRemove
 */

/**
 * @typedef {object} StaticProps
 * @property {false} [removable=false]
 */

/**
 * @typedef {object} BaseProps
 * Other properties....
 */

/**
 * @typedef {BaseProps & (RemovableProps | StaticProps) & (ParentProps | ChildlessProps)} ChipProps
 */

I was handling these conditions with propTypes, but since prop types check will be removed on React 19 I decided to try and strengthen my JSDoc so that it could handle them instead (I don't use TS).

I'm not sure there is another way to write this?

@typescript-bot
Copy link
Collaborator

This issue has been marked as "Design Limitation" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

4 participants