Skip to content

Relax visibility rules for type-aliases when 'declaration' compiler option is set. #14286

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

Open
JHawkley opened this issue Feb 24, 2017 · 6 comments
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

@JHawkley
Copy link

One common thing I use type-aliases for is to give shorter, more expressive names to commonly used types throughout a file.

This is especially useful for callback signatures that see common use throughout a module, as well as string-literal types used in a union type, like type HttpMethods = 'get' | 'post' | 'put' | 'delete'.

Let's look at the following simplified example:

type CharCallback = (c: string, i: number) => string;

export function mapOnChar(str: string, fn: CharCallback): string {
  const newStr = [];
  for (let i = 0, lim = str.length; i < lim; i++)
    newStr.push(fn(str.charAt(i), i));
  return newStr.join('');
}

When using the declaration compiler option, this example fails to compile with the following error:
error TS4078: Parameter 'fn' of exported function has or is using private name 'CharCallback'.

This initially makes sense; the type-alias is not being exported so it is private. However, if you look at the type-alias, the only thing "private" about it is the name it was given, and an alias' name is not really important to or needed for a definition file.

There is no reason that the un-exported type-alias in the example cannot be automatically de-aliased back into (c: string, i: number) => string and that used in its place when the definition file is emitted.

However, if the alias in the example were to be exported, then it should not be de-aliased in the definition file.

I should point out that this suggestion is considering type-aliases only. If the CharCallback type was re-written as interface CharCallback { (c: string, i: number): string }, then that would be a good case to raise an error. An interface is generally considered more "concrete" than a simple type-alias, and so its name should be preserved and used in the definition file.

@mhegazy
Copy link
Contributor

mhegazy commented Feb 24, 2017

Here is the problem, an ambient module has all its members automatically exported. That was a design decision we made early on to avoid clutter in declaration files (turned out not to be so wise after all:)). So for instance:

declare module "Mod" {
    var x: number;
    export function f(): void;
}

In the example above, both x and foo are exported. the export keyword is not required, nor needed.

Now to the declaration emitter. the emitter is trying to emit function mapOnChar(str: string, fn: CharCallback): string, it figures it needs CharCallback, so it writes that as well; but given the above behavior of ambient modules, this means it is making your type CharCallback public, whereas it was not. So options:

  • Do not mind, make it public any ways
    • Pros: no error reported
    • Cons: your API surface area has changed in a way you did not anticipate, moreover, you have no way of really "hiding" a declaration. Your users can now take a dependency on this type/value, and you can break them without noticing
  • Try to inline the type
    • Pros: again, no error reported
    • Cons: serializing types is not that simple, there are cases where that is not possible, e.g. classes, self-referencing types, etc.. and will make your .d.ts file look less organized, than your .ts file
  • Let the user know, and they can decide
    • Pros: either make it public, and commit to supporting it, or inline it, or not expose it at all
    • Cons: well.. an error is reported

So we picked the last option, as it seemed the safest, and most responsible.

Now for relaxing the rule for type aliases, not sure why a type alias is different from an interface, different from a class that your users will only use in a type position.. A change in any of these will break the compilation of your users regardless if they took a dependency on it.

@JHawkley
Copy link
Author

I see. But a lot of this stems off of just how "real" type-aliases are. Since I started working with TypeScript, I've been seeing them being treated as non-corporeal; they vanish as soon as you look at them. This has led me to believe that the community consensus is that this is how a type-alias should be treated by default.

They disappear in the TypeScript playground, they disappear in the IDEs, they're even known to disappear in compiler error messages. Interfaces, on the other hand, keep their name like it is holy throughout the entire code-base.

The TypeScript Handbook even makes special note of this in its "Interfaces vs Type Aliases" section:

One difference is that interfaces create a new name that is used everywhere. Type aliases don’t create a new name — for instance, error messages won’t use the alias name.

This declaration compiler option issue is the only circumstance I've found so far where a type-alias' name is treated in a first-class manner. It seems like an inconsistent application of the concept.

If I didn't make an alias public by exporting its name into my module's API, then it's either not important enough to keep its name or I made a mistake when authoring my API (and the only negative consequence would be that my API's type-definitions are a little messier than I intend, but it would still function fine, otherwise).

I know that perhaps the idea was to help people catch that kind of mistake, but I argue that this violates pre-established rules regarding type-aliases, and it's more important for the longevity and continued maintenance of the language to keep those rules until we have a really good reason to break them.

And if you really look at it, the trade-off on compromising this rule is actually not all that great. I'm basically being forced to either:

  1. Unnecessarily export simple types into my public API, making the API generally messier, but keeping my own code cleaner.
  2. In-line types that have very common use in my code, making messier and more difficult to maintain code, but cleaner APIs.

This is just not a good trade-off for the safety and responsibility you've suggested we're gaining with this. With this restraint on type-aliases, there is significant pressure to make something messy for someone somewhere.

@gcnew
Copy link
Contributor

gcnew commented Feb 25, 2017

If the type alias is not exported, how would the user declare a variable of the specific type? For me it seems that if the library author needed to make their code clean chances are the user would find it beneficial as well. I treat type aliases as part of the API surface, inlining them just pushes the problem to the consumer.

@JHawkley
Copy link
Author

At least for me, my response is, "why would I want them to declare such a variable?"

I almost exclusively use type-aliases privately or with the intention they're kept internal to my package. Most of the problems I have here are due to there not being an internal modifier for TypeScript yet, so my internally used type-aliases are sometimes forced to leak out into the public API unintentionally. I still don't want to export them, though, since they're not actually intended for public use. I would prefer they just de-alias, if it is possible for them to do so.

I can also foresee circumstances where I really do just want to de-alias away a type-alias' name even though its exposed publicly. Aliases for callbacks are probably a prime example; the function signature actually says a lot more about how the callback will be used than any name I'd give it. I still don't want to be forced to type out the whole signature every time, however.

And then, there's still the exceptional case of the odd type-alias I actually do want to make available for public consumption or I expect to have such common use throughout my code it deserves a proper name. For those, I would explicitly export them. Types like type Nullable<T> = T | null or type Trial<T> = T | Error come to mind.

This is basically what my coding practices for type-aliases have been up until I flipped on the declarations flag. These seemed like sensible ways to use them, and even probably best-practices based on what the TypeScript Handbook told me, but the current TypeScript compiler disagrees.

I would just like to have greater control over what my public API looks like. I haven't heard any really compelling arguments not to give that control back to the developer. Any possible negative consequences mentioned so far could easily be picked out by a linter and ignored if they're actually intentional.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label May 24, 2017
@spbond
Copy link

spbond commented Mar 17, 2018

I am in agreement with JHawkley. Aliases are not types, even according to the documentation, and should not be treated as types. Aliases are often used internally, for code readability. If a user of your module also requires an alias, they can make their own. However, aliases are often used in instances where it is not necessary for the user of your module to deal directly with the type definition. For example, when providing a type literal (object, callback function) or one of a union of types.

Since Aliases are not real types, they should be handled in the parser, not the compiler. A simple way to deal with the aliasing would be to keep a map of known aliases when parsing the code, and when it comes across a name, perform a lookup, replacing the alias with the thing it is aliasing. I do not see any complexities in this, it would in fact be no different than if no alias was used in the first place, and we know that that works fine. When aliasing a class or interface, you would still need to make sure the class or interface was public if needed, or you would get an error on the class or interface, not the alias, since it shouldn't really exist in the first place.

@nandin-borjigin
Copy link

nandin-borjigin commented Jul 5, 2019

I can also foresee circumstances where I really do just want to de-alias away a type-alias' name even though its exposed publicly. Aliases for callbacks are probably a prime example; the function signature actually says a lot more about how the callback will be used than any name I'd give it. I still don't want to be forced to type out the whole signature every time, however.
-- JHawkley

I can't agree more.

As @mhegazy mentioned, there are three options (which typescript chose the last for now) for solving that an unexported type being referenced by an exported one. And what we're hoping here is to choose the second one when it's possible.

I did an experiment and find out that type aliases inside a function body would be serialized in the emitted declaration file.

Take the code snippet in the OP as an example:

// source.ts
type CharCallback = (c: string, i: number) => string;
export function mapOnChar(str: string, fn: CharCallback): string {
  return '' // removed implementation detail for simplicity
}

export const mapOnChar2 = (() => {
  type CharCallback = (c: string, i: number) => string
  return function mapOnChar(str: string, fn: CharCallback): string {
    return '' // removed implementation detail for simplicity
})()

// emited source.d.ts
declare type CharCallback = (c: string, i: number) => string;
export declare function mapOnChar(str: string, fn: CharCallback): string;
export declare const mapOnChar2: (str: string, fn: (c: string, i: number) => string) => string;

Regarding @mhegazy 's concern:

not sure why a type alias is different from an interface, different from a class that your users will only use in a type position

As users of this API, if we were to use the original version and were not pretty sure about what signature it is expecting for the callback, here is how the IDE could, at best, help us:

image

While, the intellisence for the inlined version of the API would be:

image

The difference is obvious.

One might argue that users can check the .d.ts file to figure out what CharCallback really is. Then please consider this example (the usecase might seem to be particularly cooked to justify the inlining, but I assure you that it is simplified from my own real project):

// source.ts
type SomeRecursive<T extends string> = {
  [k in T]: SomeRecursive<Exclude<T, k>>
}

function factory<T extends string>(...t: T[]): SomeRecursive<T> {
  return {} as any // removed implementation detail for simplicity
}

export const foo = factory('a', 'b', 'c')

// source.d.ts
declare type SomeRecursive<T extends string> = {
    [k in T]: SomeRecursive<Exclude<T, k>>;
};
export declare const foo: SomeRecursive<"a" | "b" | "c">;

image

const factory = (() => {
  type SomeRecursive<T extends string> = {
    [k in T]: SomeRecursive<Exclude<T, k>>
  }
  return function<T extends string>(...t: T[]): SomeRecursive<T> {
    return {} as any // removed implementation detail for simplicity
  }
})()

export const bar = factory('a', 'b', 'c')

// source.d.ts
export declare const bar: {
    a: {
        b: {
            c: {};
        };
        c: {
            b: {};
        };
    };
    b: {
        a: {
            c: {};
        };
        c: {
            a: {};
        };
    };
    c: {
        a: {
            b: {};
        };
        b: {
            a: {};
        };
    };
};

image


I am aware that under certain circumstances (I ran into one myself earlier), serializing the type could be expensive or even impossible. But we don't really need the inlining everywhere and those occurrences where we expect inlinings are most likely simple ones. So wherever the compiler fails to perform an inlinging, it can fall back to option 3 and seek for a human intervention.

@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 and removed Needs Investigation This issue needs a team member to investigate its status. labels Aug 22, 2019
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

No branches or pull requests

6 participants