search

microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.95k stars 12.47k forks source link

Support for all type features in declaration files. #35822

Open trusktr opened 4 years ago

trusktr commented 4 years ago

I'm on TS 3.7.3 at time of writing this.

It seems like this issue should simply be focused on and fixed before continuing to add more and more features to the language and widening the source vs declaration gap.

This is quite a problem!

Search terms

"typescript declaration file limitations" "has or is using private name" "exported class expression may not be private or protected" "Property 'foo' of exported class expression may not be private or protected. ts(4094)" "Return type of exported function has or is using private name 'foo'. ts(4060)"

Related issues (a small fraction of the results on Google) in no particular order:

Workaround

One way to work around all of the above issues, for libraries, is to have library authors point their types field in package.json to their source entrypoint. This will eliminate the problems in the listed issues, but has some big downsides:

  1. If a downstream consumer of a library without declaration files (f.e. without dist/index.d.ts) but with types pointing to a source file (f.e. src/index.ts) then if the consumer's tsconfig.json settings are not the same as the library's, this may cause type errors (f.e. the library had strict set to false while being developed, but the consumer sets strict to true and TypeScript picks up all the strict type errors in the library code).
  2. If skipLibCheck is set to true, the consumer project's compilation will still type-check the (non-declaration) library code regardless.

With those two problems detailed, if you know you will be working in a system where you can guarantee that all libraries and consumers of those libraries will be compiled with the same compiler (tsconfig.json) settings (i.e. similar to Deno that defines compiler options for everyone), then pointing the package.json types field to a source file (because declaration output is not possible) is currently the workaround that allows all the unsupported features of declaration files to be usable. But this will fail badly if a library author does not know what compiler settings library consumers will have: everything may work fine during development and testing within external examples, but a downstream user will eventually report type errors if they compile their program with different settings while depending on the declaration-less libraries!

Suggestion

Support for all type features in declaration files.

Please. 🙏 I love you TS team, you have enabled so much better code dev with TypeScript. :heart: :blush: TS just needs declaration love for support of all features of the language.

Some work is being done, f.e. https://github.com/microsoft/TypeScript/issues/23127, but overall I feel that too much effort is being put onto new language features while declaration output is left behind.

This creates a poor developer experience when people's working code doesn't work (the moment they wish to publish it as a library and turn on declaration: true).

I hope the amazing and wonderful TS team realize how frustrating it may feel for someone to work on a project for months, only to end with un-fixable type errors the moment they want to publish the code as a library with declaration: true, then having to abandon features and re-write their code, or try to point package.json "types" to .ts files only to face other issues.

I wish every new feature of the language came paired with working tests for equivalent declaration emit. Can this be made a requirement for every new language feature, just like unit tests are a requirement?

Use Case

Use any language feature, publish your code, then happily move along, without facing issues like

src/html/DeclarativeBase.ts:25:10 - error TS4094: Property 'onChildSlotChange' of exported class expression may not be private or protected.

25 function makeDeclarativeBase() {
            ~~~~~~~~~~~~~~~~~~~

src/html/WebComponent.ts:32:10 - error TS4060: Return type of exported function has or is using private name 'WebComponent'.

32 function WebComponentMixin<T extends Constructor<HTMLElement>>(Base: T) {
            ~~~~~~~~~~~~~~~~~

I worked hard for months to get the typings in the above code working, then I turned on "declaration": true and TypeScript said "today's not the day!".

I hope you the team can see that these issues are a pain, and realize the need to bring declaration emit to parity with language features and not letting the gap between language features and declaration output widen.

Examples

The issue that sparked me to write this was https://github.com/microsoft/TypeScript/issues/35744.

In that issue there's an example of what implicit return types might look like:

export declare function AwesomeMixin<T extends Constructor>(Base: T) {
  type Foo = SomeOtherType<T> // only types allowed in here.

  // only special return statements allowed, or something.
  return declare class Awesome extends Base {
    method(): Foo
  }
}

We'd need solutions for other problems like the protected/private error above, etc.

Of course it'll take some imagination, but more importantly it will take some discipline: disallow new features without declaration parity.

Checklist

My suggestion meets these guidelines:

trusktr commented 4 years ago

There were too many issues to reference, so I stopped. But I hope this shows how much of a time waster this problem really is.

evmar commented 4 years ago

Can you describe what the desired return type (to be used in the d.ts) of your example is?

trusktr commented 4 years ago

@evmar This is a good description:

The return type is class Awesome extends Base { method(): Foo } where Base is determined from the generic type arg T that is inferred (or passed) at the call site of the AwesomeMixin function.

I believe declaration files could be made to support this.

cdata commented 4 years ago

Personally I find #30355 to be a sharp edge for anyone who hopes to publish a library that will be used by folks outside the TypeScript ecosystem. In general it seems unwholesome that declaration files are unable to express types with parity to .ts sources.

RyanCavanaugh commented 4 years ago

We have a self-imposed design constraint that we do not produce new named types in definition files. This is critical because it ensures that no one is taking a dependency on names that might change in the future!

If you have specific examples where you can show unambiguous code that should have some certain .d.ts emit that behaves under that constraint, please post them.

As a workaround today; we have a flag that will let you know when TS isn't able to produce a declaration file for your code.

cdata commented 4 years ago

Thank you for the feedback @RyanCavanaugh .

With respect, this design constraint seems to impose a double standard across consumers of .ts sources and .d.ts+.js sources.

Here is one of my strawperson use cases: vending mixins. It is probably one that you have heard before. In this basic case, I can work with .ts sources directly and everything is fine, but if I generate .d.ts I'm told that a private/protected member is not allowed in the class generated by the mixin:

playground

interface Constructor<T> {
    new(...args: any[]): T;
}

class Foo { 
    foo: boolean = true;
}

const BarMixin = <T extends Constructor<Foo>>(FooClass: T) => {
    class FooBar extends FooClass {
        protected bar: boolean = true;
    }

    return FooBar;
};

type BarInterface = InstanceType<ReturnType<typeof BarMixin>>;

AFAIK this is not producing a new named type (although perhaps I have an incorrect understanding of what name means in this context), but none-the-less falls into the category of disparity in how types are treated between .ts and .d.ts files. It's also notable that this issue arises only when leveraging API access modifiers in the scoped class.

liamross commented 4 years ago

I'm trying to publish a React component library. I have a utility hook that prevents me from setting "declaration": true, which is absolutely essential since this will be used in TypeScript projects:

The hook

import { MouseEvent, MouseEventHandler, useCallback } from 'react';

export default function useOnClick<T>(onClick?: MouseEventHandler<T>) {

  const handler = useCallback((event: MouseEvent<T>) => {
      // does some stuff
  }, []);

  return handler;
}

The error

src/hooks/useOnClick.ts:16:25 - error TS4060: Return type of exported function has or is using private name 'MouseEvent'.

16 export default function useOnClick<T>(onClick?: MouseEventHandler<T>) {
                           ~~~~~~~~~~

I'm confused as to why a type from React is preventing me from publishing types within my package.

evmar commented 4 years ago

Try writing down the return type of your function. (That is what the d.ts will contain, after all.)

trusktr commented 4 years ago

@evmar If the return type is a class, you can not write the return type manually. class produces special types that can only be written with class syntax at the moment.

@RyanCavanaugh

We have a self-imposed design constraint that we do not produce new named types in definition files. This is critical because it ensures that no one is taking a dependency on names that might change in the future!

This obviously doesn't behave like regular .ts files, which is strange. Why have a certain constraint only in one environment (consuming .d.ts + .js files) and not the other (consuming .ts files)?

liamross commented 4 years ago

Try writing down the return type of your function. (That is what the d.ts will contain, after all.)

Weird, that actually worked... Shouldn't the inferred return type be emitted in the d.ts without the manual adding of return types?

evmar commented 4 years ago

@trusktr My point is, if you cannot find a way to write the return type manually, then the compiler doesn't have any way of generating the return type for you.

@liamross Is is possible the explicit type you wrote is different from the inferred type that the compiler wanted to generate?

trusktr commented 4 years ago

My point is, if you cannot find a way to write the return type manually, then the compiler doesn't have any way of generating the return type for you.

Exactly! That's why I opened this feature request with the title "Support for all type features in declaration files.", so that functionality can be added (be it new syntax in declaration files, or something else).

evmar commented 4 years ago

I now see why I didn't follow you! What you're asking for is inexpressible in .ts files too, in that you can't write the type in a .ts or a .d.ts file. So the issue isn't really tied to declaration files, except to the extent that they require you to write types. For a (contrived) example of the difference, your code won't be usable by a documentation generator either (since it also will want to show types in the docs).

RyanCavanaugh commented 4 years ago

I am leaning toward declaring this unactionable due to lack of a concrete request. There are a wide variety of type system features; basically what's being requested here is either that we make the type system in implementation space actively worse to maintain the "all .ts can produce .d.ts" guarantee, which is just not the right way to make good software, or it's a request to add every type system feature needed to produce these representations, which isn't actionable. Named type expressions would probably go 80% of the way there, but what's the remaining 20%? There aren't a lot of specifics here and this isn't just a "go fix it" bug report.

cdata commented 4 years ago

@RyanCavanaugh this seems to be a meta bug that aggregates all of the ways in which .d.ts cannot be generated for corresponding .ts.

I don't think anyone wants to reduce the capabilities of .ts in order to satisfy this issue. My read is that folks want .d.ts files to be able to faithfully represent as close to 100% of the type information contained in a corresponding .ts file as possible.

it's a request to add every type system feature needed to produce these representations, which isn't actionable

Personally I think it's actionable, but it's a very large task that should probably be broken down into sub-tasks.

With your insight, perhaps you could help us by suggesting existing sub-tasks that lead to a happier outcome for concerned users? It sounds like there is one already ("named type expressions") - is there a corresponding issue that can be referenced? After reading it, we might be able to work together to uncover the remaining 20% and file issues as needed 👍

cdata commented 4 years ago

I don't know if it will be helpful, but I would like to offer this anecdote:

At my company (Google), TypeScript source must be transpiled to .js+.d.ts as an intermediate step in the mainstream JS build pipeline. This means that any type-related feature that is not supported in a declaration file is unavailable to me and my TypeScript-authoring colleagues.

RyanCavanaugh commented 4 years ago

30979

trusktr commented 4 years ago

It may be too late for this, or maybe it is a lot of effort in refactoring declaration code, but wouldn't it be easier if declaration files were simply source files stripped of all source, leaving behind only types?

For example, take this mixin:

interface Constructor<T> {
    new(...args: any[]): T;
}

export class Foo { 
    foo: boolean = true;
}

type MyNumber = number

let counter: MyNumber = 0

export const BarMixin = <T extends Constructor<Foo>>(FooClass: T) => {
    console.log('making FooBar class #', ++counter)

    return class FooBar extends FooClass {
        protected bar: boolean = true;
    }
};

it would be converted to something like

interface Constructor<T> {
    new(...args: any[]): T;
}

export class Foo { 
    foo: boolean/* = true .. implementation stripped .. */;
}

/* .. implementation stripped ..
type MyNumber = number

let counter: MyNumber = 0
*/

// no "declare" keyword here, for simplicity. We're already in a declaration `.d.ts` file, so it is implied.
export const BarMixin = <T extends Constructor<Foo>>(FooClass: T) => {
    // console.log('making FooBar class #', ++counter) .. implementation stripped ..

    // the "return" here is used only to define the return type. Simple!
    return class FooBar extends FooClass {
        protected bar: boolean/* = true .. implementation stripped .. */;
    }
};

You see how simple that is? The .d.ts file expresses all of the types in the .ts file.

Are you concerned that code will rely on the private FooBar type? Isn't that.... the point?

It seems to me that this would greatly reduce the TypeScript implementation complexity, because now declaration types would rely on the same exact type system as source types.

trusktr commented 4 years ago

Is #30979 the feature that will cover the case of the class types returned from a functions?

I think the implementation for that feature wouldn't be needed if declaration files were equivalent of source .ts files stripped of runtime implementation (and stripped of types that aren't used in publicly exposed interfaces). But it may be a huge effort to reollback and down that path.

Just imagine: for ever new .ts feature, a parallel declaration feature wouldn't need to be implemented and maintained. The implementation details are simply stripped.

As an alternative to the above idea of re-writing the declaration implementation, what if we add an option like declarationsUseSource, that simply copies .ts files to the output dir, instead of generating delcaration files?

It would be similar to pointing types in package.json to .ts files instead of .d.ts files, which works in many cases, except for issues like https://github.com/microsoft/TypeScript/issues/35744 which would need to be solved somehow.

trusktr commented 4 years ago

I was curious about the (somewhat) new composite feature, and discovered that composite projects must have declaration set to true. This can cause all of the above issues in composite projects.

lifeiscontent commented 4 years ago

@RyanCavanaugh I have a large React UI Library written in TypeScript I'd really not like to export all types for props as there is a clear method for grabbing props of a component. Here's my example:

interface Props {
  children: React.ReactNode;
  disabled?: boolean;
  fullWidth?: boolean;
  intent?: 'primary' | 'secondary' | 'tertiary' | 'neutral' | 'destructive';
  onClick?: React.MouseEventHandler<HTMLButtonElement>;
  size?: 'smallest' | 'small' | 'medium' | 'large' | 'largest';
}

export function UIButton(props: Props): JSX.Element {
  return (
    <button
      className={clsx('UIButton', {
        'UIButton--disabled': props.disabled,
        'UIButton--full-width': props.fullWidth,
        'UIButton--intent-destructive': props.intent === 'destructive',
        'UIButton--intent-neutral': props.intent === 'neutral',
        'UIButton--intent-primary': props.intent === 'primary',
        'UIButton--intent-secondary': props.intent === 'secondary',
        'UIButton--intent-tertiary': props.intent === 'tertiary',
        'UIButton--size-large': props.size === 'large',
        'UIButton--size-largest': props.size === 'largest',
        'UIButton--size-medium': props.size === 'medium',
        'UIButton--size-small': props.size === 'small',
        'UIButton--size-smallest': props.size === 'smallest',
      })}
      disabled={props.disabled}
      onClick={props.onClick}
    >
      {props.children}
    </button>
  );
}

and if people need access to the props, they can do so by using React.ComponentPropsWithoutRef<typeof UIButton>. This is a clear example I think for use of private interfaces because its still accessible, without having to deal with exporting the types explicitly.

The reason why this is an issue for me, is I have a single index file where I export * from './UIButton'; and every other component in my library, there would be a collision of types here, and I'd really rather not rename all my types to be prefixed with the component name.

trusktr commented 4 years ago

The changes in https://github.com/microsoft/TypeScript/pull/32028 can lead to situations where code works perfectly fine in the editor, then when ready to publish a library, it all breaks when trying to make declaration files (due to the problems in the OP list of issues). (cc @sheetalkamat)

jeffryang24 commented 3 years ago

I also got error TS4060: Return type of exported function has or is using private name 'WithLocale' when using hoist-non-react-statics.

This normal pattern cause error when declaration: true:

export default function withLocaleComponent<
  TProps extends WithLocaleComponentInjectedProps
>(Component: React.ComponentType<TProps>) {
  class WithLocale extends React.Component<
    Omit<TProps, keyof WithLocaleComponentInjectedProps> &
      WithLocaleComponentProps
  > {
    static displayName = getDisplayName("withLocale", Component);

    render() {
      return (
        <LocaleContext.Consumer>
          {context => {
            let locale = this.props.locale || context.locale;
            if (typeof locale === "string") {
              const [language, country] = locale.split("-");
              locale = { country, language };
            }
            return (
              <Component
                {...(this.props as TProps)}
                {...context}
                locale={locale}
              />
            );
          }}
        </LocaleContext.Consumer>
      );
    }
  }

  return hoistStatics(WithLocale, Component);
}

Then, I got an idea to create a function mixin that return a class and no error was occurred with declaration: true.

function withLocaleMixin<TProps extends WithLocaleComponentInjectedProps>(
  Component: React.ComponentType<TProps>
) {
  return class WithLocale extends React.Component<
    Omit<TProps, keyof WithLocaleComponentInjectedProps> &
      WithLocaleComponentProps
  > {
    static displayName = getDisplayName("withLocale", Component);

    render() {
      return (
        <LocaleContext.Consumer>
          {context => {
            let locale = this.props.locale || context.locale;
            if (typeof locale === "string") {
              const [language, country] = locale.split("-");
              locale = { country, language };
            }
            return (
              <Component
                {...(this.props as TProps)}
                {...context}
                locale={locale}
              />
            );
          }}
        </LocaleContext.Consumer>
      );
    }
  };
}

export default function withLocaleComponent<
  TProps extends WithLocaleComponentInjectedProps
>(Component: React.ComponentType<TProps>) {
  return hoistStatics(withLocaleMixin<TProps>(Component), Component);
}

My question is, is it ok to use the second pattern (with withLocaleMixin method which return a class)? Or, it will cause a new behavior than the first one? :bow:

trusktr commented 3 years ago

I updated the original post with a Workaround section that describes in more detail how to currently work around all the listed issues, but it comes with fairly big other downsides.

canonic-epicure commented 3 years ago

Keeping the discussion going, copied from #44360:

I found it very discouraging, that TypeScript is currently fragmented into 2 languages. The 1st one is TypeScript itself and 2nd is TypeScript+declaration files. The program, valid in the 1st language, is not valid in the 2nd. This means, the TypeScript is a superset of TypeScript+declarations.

You can express more logic in TypeScript, however the TypeScript+declarations is a standard for distributing code (to avoid recompilation of the source files).

Despite being a well-known issue, this fragmentation is not documented anywhere, and TS development team does not provide much feedback about it. It is a hushed up thing, nobody wants to talk about this. This issue should, at the very least, be described in details, on the documentation website.

The solution is simple - a design decision should be made, that *.d.ts files should create exactly the same internal compilation data, as regular source files. For that, probably a different format of the declaration files is needed, *.d2.ts

Its very discouraging, when you have to spent hours, mingling your code (that compiles just fine in TypeScript), reducing type-safety, to make it work in the TypeScript+declarations language. In many cases its not possible at all.

saschanaz commented 3 years ago

Documenting the difference sounds good, maybe somewhere here? https://www.typescriptlang.org/docs/handbook/declaration-files/introduction.html

canonic-epicure commented 3 years ago

Yes, looks like an appropriate place. Just need to make it clear, that only a subset of regular TypeScript features is supported with declarations files emit.

canonic-epicure commented 3 years ago

As suggested by @trusktr in the thread, changing the way of declaration files generation to just stripping the implementation and keeping only types if probably the best solution to the problem.

Meanwhile, even if heading to the route of fixing the existing solution, it seems like most pieces of the puzzle are already there.

As mentioned in the thread, anonymous types can be solved with https://github.com/microsoft/TypeScript/pull/30979 That is, "green", ready-to-merge PR.

The other part is related to the fact, that the type of internal mixin class is represented with the object. Of course that would lead to error. For example, for this mixin:

export type AnyFunction<A = any> = (...input: any[]) => A
export type AnyConstructor<A = object> = new (...input: any[]) => A
export type Mixin<T extends AnyFunction> = InstanceType<ReturnType<T>>

export const MyMixin = <T extends AnyConstructor>(base : T) =>
    class MyMixin extends base {
        method () : this {
            return this
        }
    }
export type MyMixin = Mixin<typeof MyMixin>

The generated declaration is:

export declare type AnyFunction<A = any> = (...input: any[]) => A;
export declare type AnyConstructor<A = object> = new (...input: any[]) => A;
export declare type Mixin<T extends AnyFunction> = InstanceType<ReturnType<T>>;
export declare const MyMixin: <T extends AnyConstructor<object>>(base: T) => {
    new (...input: any[]): {
        method(): this;
    };
} & T;
export declare type MyMixin = Mixin<typeof MyMixin>;

Which leads to error this is only available in the non-static method of the class

But, there's another "green" PR: https://github.com/microsoft/TypeScript/pull/41587 which can be used to fix this. This PR should also fix the well-known problem of private/protected methods. With this PR, the declarations for the MyMixin would look like:

export declare const MyMixin: <T extends AnyConstructor<object>>(base: T) => typeof class MyMixin {
    new (...input: any[]): MyMixin;
    method(): this;
} & T;

So I believe, fixing this ticket is a matter of proper prioritization and there's no irresistible technical problems in it.

This issue clearly causes a lot of struggle for a lot of people, and there's clearly no intention, to even discuss the ways of fixing it. Dear TS team, why this attitude?

frank-weindel commented 3 years ago

@RyanCavanaugh I have to say, I'm also disheartened by this. A team could spend a long time building a library and assuming that "if it compiles, it'll work!" only to find out after turning declarations on that they have a ton of very difficult to solve errors. It's not intuitive and not very much documented that there are such limitations. I'm happy I'm finding this out now by chance.