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

Applying a constrained generic with type parameters as the argument type doesn't properly catch constrained fields #52309

Closed
shapirone opened this issue Jan 19, 2023 · 6 comments
Labels
Duplicate An existing issue was already created

Comments

@shapirone
Copy link

shapirone commented Jan 19, 2023

Bug Report

🔎 Search Terms

generic constraints, generic types, multiple generics, generic type variable, type parameters

🕗 Version & Regression Information

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

⏯ Playground Link

Playground link with relevant code

💻 Code

type PropertiesType = {[key:string]: any};
type RequiredPropertiesType<Props extends PropertiesType> = (Exclude<keyof Props, symbol>)[];

interface JSONSchemaObjectType<
  Props extends PropertiesType,
  RequiredProps extends RequiredPropertiesType<Props>
> {
  properties: Props;
  required?: RequiredProps;
  additionalProperties?: boolean;
}

const DefineObject = <
  Props extends PropertiesType,
  RequiredProps extends RequiredPropertiesType<Props>,
  Def extends JSONSchemaObjectType<Props, RequiredProps>
>(definition: Def) => {
  // noop
}

DefineObject({
  properties: {
    test: "hi"
  },
  required: ["test", "broken like me"] // <-- no static type error
})

🙁 Actual behavior

The required array accepts values that are not keys used in it's sibling properties

🙂 Expected behavior

The required array is constrained to the keys used in it's sibling properties

@whzx5byb
Copy link

You should explicitly provide the constraint as a context to help infer the correct type of Props:

const DefineObject = <
  Props extends PropertiesType,
  RequiredProps extends RequiredPropertiesType<Props>,
  Def extends JSONSchemaObjectType<Props, RequiredProps>
- >(definition: Def) => {
+ >(definition: Def & JSONSchemaObjectType<Props, RequiredProps>) => {
}

@shapirone
Copy link
Author

@whzx5byb this constrains the argument but causes any conditional typing that relies on definition being a generic to fail

@RyanCavanaugh
Copy link
Member

this constrains the argument but causes any conditional typing that relies on definition being a generic to fail

Can you go into more detail? The above solution is effectively correct; there's not a way to exclude candidates from inference like you would need in order to make this to work.

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Feb 2, 2023
@RyanCavanaugh
Copy link
Member

Treating as duplicate of #14829

@shapirone
Copy link
Author

Sorry for the silence here, I've been looking into the resulting conditional typing failure and haven't been able to reproduce what we're seeing in a simplified TS Playground. Applying the recommended solution appropriately applied the constraint on the required field, but caused static type test failures that I assumed were related to inference from our conditional typing.

I've since realized that the test failures we're seeing are resolved if we assign the result of DefineObject to a variable and reference that variable in the surrounding function call, rather than using DefineObject directly as the object value. Code sample below to illustrate

// This causes test failures in our inferred runtime typing
DefineFunction({
  input_parameters: {
    properties: {
      myObject: DefineObject({
        type: Schema.types.object,
        properties: { req: { type: "string" }, opt: { type: "string" } },
        required: ["req"],
      });
    },
    required: ["myObject"],
  },
});


// This does not
const MyObject = DefineObject({
  type: Schema.types.object,
  properties: { req: { type: "string" }, opt: { type: "string" } },
  required: ["req"],
});
  
DefineFunction({
  input_parameters: {
    properties: {
      myObject: MyObject
    },
    required: ["anObject"],
  },
});

I'm still working on providing a Playground example, but please let me know if this behavior is familiar to you, if I should close this issue and open a new issue when I have more details, or if a pointer to where this creates problems in our actual code base would be helpful 🙇

@typescript-bot
Copy link
Collaborator

This issue has been marked as a 'Duplicate' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants