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

Support for all type features in declaration files. #35822

Open
5 tasks done
trusktr opened this issue Dec 21, 2019 · 29 comments
Open
5 tasks done

Support for all type features in declaration files. #35822

trusktr opened this issue Dec 21, 2019 · 29 comments
Labels
Meta-Issue An issue about the team, or the direction of TypeScript

Comments

@trusktr
Copy link
Contributor

trusktr commented Dec 21, 2019

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. ❤️ 😊 TS just needs declaration love for support of all features of the language.

Some work is being done, f.e. #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 #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:

  • 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, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.
This was referenced Dec 21, 2019
@lifeiscontent
Copy link

lifeiscontent commented Jul 26, 2020

@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
Copy link
Contributor Author

trusktr commented Sep 21, 2020

The changes in #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
Copy link

jeffryang24 commented Mar 14, 2021

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? 🙇

kamilmielnik added a commit to prezly/slate that referenced this issue Mar 19, 2021
    ERROR in (..)/node_modules/@prezly/slate-editor/build/components/FloatingMenu/index.d.ts(1,25):
    TS2307: Cannot find module './FloatingMenu' or its corresponding type declarations.
    ERROR in (..)/node_modules/@prezly/slate-editor/build/components/LoadingPlaceholderV2/index.d.ts(1,25):
    TS2307: Cannot find module './LoadingPlaceholderV2' or its corresponding type declarations.

See: microsoft/TypeScript#35822
kamilmielnik added a commit to prezly/slate that referenced this issue Mar 19, 2021
    ERROR in (...)/node_modules/@prezly/slate-editor/build/components/TooltipV2/index.d.ts(1,32):
    TS2307: Cannot find module './TooltipV2' or its corresponding type declarations.

See: microsoft/TypeScript#35822
@trusktr
Copy link
Contributor Author

trusktr commented Apr 19, 2021

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.

trusktr added a commit to lume/eventful that referenced this issue Apr 19, 2021
…hat we can output declaration files

lowclass/Mixin and TypeScript "private" or "protected" access modifiers cause declaration output not to be possible, thus forcing us to point package.json types at src/index.ts instead of dist/index.d.ts, which can cause type errors in downstream consumers if they use different tsconfig.json settings when they type-check their applications (coupled with the fact that TypeScript will type-check library code if the library code types field does not point to declaration files, *even if* the user set `skipLibCheck` to `true`). Outputting declaration files and pointing `types` to them is the way to go for publishing libraries, but certain features of TypeScript do not work when declaration output is enabled, which is why we had to refactor. microsoft/TypeScript#35822

Now that lume/cli is updated, npm test fails. Needs a fix.
trusktr added a commit to lume/eventful that referenced this issue Apr 19, 2021
… fields such that we can output declaration files

lowclass/Mixin and TypeScript "private" or "protected" access modifiers cause declaration output not to be possible, thus forcing us to point package.json types at src/index.ts instead of dist/index.d.ts, which can cause type errors in downstream consumers if they use different tsconfig.json settings when they type-check their applications (coupled with the fact that TypeScript will type-check library code if the library code types field does not point to declaration files, *even if* the user set `skipLibCheck` to `true`). Outputting declaration files and pointing `types` to them is the way to go for publishing libraries, but certain features of TypeScript do not work when declaration output is enabled, which is why we had to refactor. microsoft/TypeScript#35822

Now that lume/cli is updated, npm test fails. Needs a fix.
@canonic-epicure
Copy link

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
Copy link
Contributor

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

@canonic-epicure
Copy link

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
Copy link

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 #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: #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
Copy link

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Meta-Issue An issue about the team, or the direction of TypeScript
Projects
None yet
Development

No branches or pull requests

10 participants