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

Allow mapped type to declare functions #48125

Open
5 tasks done
rockwalrus opened this issue Mar 4, 2022 · 10 comments
Open
5 tasks done

Allow mapped type to declare functions #48125

rockwalrus opened this issue Mar 4, 2022 · 10 comments
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@rockwalrus
Copy link

Suggestion

πŸ” Search Terms

  • mapped types
  • instance member function
  • instance member property
  • instance method
  • subclass constructor
  • overriding

βœ… Viability Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

πŸ“ƒ Motivating Example

In our universe, there are two kinds of animals, dogs and cats.

type Animal = 'dog' | 'cat';

Some people can handle dogs. Some people can handle cats. Some people can even handle both. But if you can't handle dogs or cats, you're just not a handler. We can express this generically in TypeScript.

type Handler<A extends Animal> = {[animal in A as `handle${Capitalize<animal>}`]: (animal: animal) => void};

Let's say you have a dog handler that doesn't do anything interesting.

class BoringDogHandler implements Handler<'dog'> {
	handleDog(dog : `dog`) {}
}

You could also have someone who can handle both and keeps a record of how many animals handled.

class CountingUniversalAnimalHandler implements Handler<Animal> {
	count = 0;

	handleDog() {this.count++;}
	handleCat() {this.count++;}
}

Some dog handlers are well known for broadcasting to the world whenever they are handling a dog. Since this is such a common thing, we'd like to create a mixin for it.

function NoisyDogHandler<H extends {new (...args: any[]): Handler<'dog'>}>(base: H) {
	return class extends base {
		handleDog(dog : `dog`) {
			console.log("Handling dog");
			return super.handleDog(dog);
		}
	};
}

Looks innocuous enough. But this doesn't work! Typescript informs us that

Class Handler<"dog"> defines instance member property handleDog, but extended class (Anonymous class) defines it as instance member function. (2425)

If we were allowed to declare that the handle${Capitalize<A>} members were not mere properties, but actually methods, TypeScript would not fear letting us override them. The natural syntax would look like this, which is currently not allowed:

type Handler<A extends Animal> = {[animal in A as `handle${Capitalize<animal>}`](animal: animal): void};

A subtle distinction for a subtle distinction.

Playground Example

πŸ’» Use Cases

What do you want to use this for?

Mix-ins are powerful, and when they work they're fantastic, but they're currently somewhat brittle to work with.

This is one of the pieces of the puzzle needed for classes extending constructors of mapped types to override methods (#27689). The other piece needed is to be able to keep the member function status of methods obtained from type indexing (#38496, c.f. #35416, and #46802).

What shortcomings exist with current approaches?

For small enough use cases, you can do the mapping "by hand":

type Animal = 'dog' | 'cat';

type DogHandler = {handleDog(dog: 'dog'): void};
type CatHandler = {handleCat(cat: 'cat'): void};
type BothHandler = DogHandler & CatHandler;

type Handler = DogHandler | CatHandler;
type HandlerFor<A extends Animal> = ('dog' extends A ? DogHandler : {}) & ('cat' extends A ? CatHandler : {})

Obviously this doesn't scale well if you create multiple methods with Animal instead of just one, or Animal has many more cases. Less obviously, there isn't a really satisfactory way to implement a generic method that takes an animal and handler, calls the right handle method, ensures at compile time that the handler can handle the animal, and doesn't involve casting. See this playground for several attempts.

@RyanCavanaugh
Copy link
Member

Based on the use case I'm inclined to just discard the ... defines instance member property ..., but extended class ... defines it as instance member function. error when the base class property is created through a mapped type. Thoughts?

@fatcerberus
Copy link

fatcerberus commented Mar 5, 2022

Keep in mind that, assuming this proposed syntax does create methods, TS would likely treat them bivariantly, meaning you could potentially unsoundly assign a CatHandler to a DogAndCatHandler (I bring this up only because the expected subtyping relationship for handlers is explicitly mentioned in the OP).

See https://github.com/microsoft/TypeScript/wiki/FAQ#why-are-function-parameters-bivariant and note that strictFunctionTypes enables contravariant parameters for function types, but not methods.

@rockwalrus
Copy link
Author

Based on the use case I'm inclined to just discard the ... defines instance member property ..., but extended class ... defines it as instance member function. error when the base class property is created through a mapped type. Thoughts?

That sounds reasonable to me.

@rockwalrus
Copy link
Author

Oh, but if you do discard the error message, make sure it discards it for subclasses as well.

@RyanCavanaugh RyanCavanaugh added In Discussion Not yet reached consensus Suggestion An idea for TypeScript labels Mar 8, 2022
@frank-weindel
Copy link

frank-weindel commented Jul 27, 2022

Dealing with this as well. Removing the error for this situation sounds good. I'd go as far to say that anytime an instance member property can be either a single method signature (or undefined) then it should be overridable as if it was a method.

@eliasm307
Copy link

eliasm307 commented Aug 6, 2022

Yes would be good if this wasnt an error.

I'm creating a util to infer class types from some legacy code I'm working with that creates custom ES6 compatible classes.

The util uses mapped types and this error means a lot of ts-ignores need to be used when overriding methods in ES6 classes from these inferred classes in TS

@derekdon
Copy link

derekdon commented Aug 8, 2023

Have the same problem.

@adrien2p
Copy link

I have the same problem as well πŸ‘

@adrien2p
Copy link

adrien2p commented Jan 23, 2024

Here is my case

export type AbstractModuleService<
  TContainer,
  TMainModelDTO,
  TOtherModelNamesAndAssociatedDTO extends OtherModelsConfigTemplate
> = AbstractModuleServiceBase<TContainer, TMainModelDTO> & {
  [K in keyof TOtherModelNamesAndAssociatedDTO as `retrieve${ExtractSingularName<
    TOtherModelNamesAndAssociatedDTO,
    K
  > &
    string}`]: (
    id: string,
    config?: FindConfig<any>,
    sharedContext?: Context
  ) => Promise<TOtherModelNamesAndAssociatedDTO[K & string]["dto"]>
} & {
  [K in keyof TOtherModelNamesAndAssociatedDTO as `list${ExtractPluralName<
    TOtherModelNamesAndAssociatedDTO,
    K
  > &
    string}`]: (
    filters?: any,
    config?: FindConfig<any>,
    sharedContext?: Context
  ) => Promise<TOtherModelNamesAndAssociatedDTO[K & string]["dto"][]>
} & {
  [K in keyof TOtherModelNamesAndAssociatedDTO as `listAndCount${ExtractPluralName<
    TOtherModelNamesAndAssociatedDTO,
    K
  > &
    string}`]: {
    (filters?: any, config?: FindConfig<any>, sharedContext?: Context): Promise<
      [TOtherModelNamesAndAssociatedDTO[K & string]["dto"][], number]
    >
  }
} & {
  [K in keyof TOtherModelNamesAndAssociatedDTO as `delete${ExtractPluralName<
    TOtherModelNamesAndAssociatedDTO,
    K
  > &
    string}`]: {
    (
      primaryKeyValues: string[] | object[],
      sharedContext?: Context
    ): Promise<void>
  }
} & {
  [K in keyof TOtherModelNamesAndAssociatedDTO as `softDelete${ExtractPluralName<
    TOtherModelNamesAndAssociatedDTO,
    K
  > &
    string}`]: {
    <TReturnableLinkableKeys extends string>(
      primaryKeyValues: string[] | object[],
      config?: SoftDeleteReturn<TReturnableLinkableKeys>,
      sharedContext?: Context
    ): Promise<Record<string, string[]> | void>
  }
} & {
  [K in keyof TOtherModelNamesAndAssociatedDTO as `restore${ExtractPluralName<
    TOtherModelNamesAndAssociatedDTO,
    K
  > &
    string}`]: {
    <TReturnableLinkableKeys extends string>(
      productIds: string[] | object[],
      config?: RestoreReturn<TReturnableLinkableKeys>,
      sharedContext?: Context
    ): Promise<Record<string, string[]> | void>
  }
}

Where I expect the mapped types to be functions of the class and not properties as it is only possible to define them as of now, the objective is to allow classic override of the method from the child class. But being properties (since there is a limitation), the only way I found around it is to override it like the following

  @InjectManager("baseRepository_")
  async listVariants_(
    filters: ProductTypes.FilterableProductVariantProps = {},
    config: FindConfig<ProductTypes.ProductVariantDTO> = {},
    @MedusaContext() sharedContext: Context = {}
  ): Promise<ProductTypes.ProductVariantDTO[]> {
    const variants = await this.productVariantService_.list(
      filters,
      config,
      sharedContext
    )

    return JSON.parse(JSON.stringify(variants))
  }

  get listVariants() {
    return this.listVariants_
  }

any news in terms of advancement on that topic?

@BalaM314
Copy link

Same issue, would be nice to just remove that error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

8 participants