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

TS 3.7 generates inline imports for typing files (.d.ts) although the source contains imports at the top #36097

Closed
CarstenLeue opened this issue Jan 9, 2020 · 6 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@CarstenLeue
Copy link

This is a follow up to #30258. I am now using TS 3.7.4 and the compiler sometimes generates inline imports although the TS source contains imports in the head area of the file.

Example:

TS Source is:

import {
  AbstractRegisteredComponent,
  AbstractRegisteredLayoutMapping
} from '@acoustic-content-sdk/component-utils';
import { ComponentTypeRef } from '@acoustic-content-sdk/react-api';

import { LayoutComponentDirective } from './layout.directive';

export type RegisteredComponent = AbstractRegisteredComponent<
  ComponentTypeRef,
  LayoutComponentDirective
>;

export type RegisteredLayoutMapping = AbstractRegisteredLayoutMapping<
  ComponentTypeRef
>;

const KEY_COMPONENT = Symbol();
const KEY_LAYOUT_MAPPING = Symbol();

export const registerComponent = (
  aType: ComponentTypeRef,
  aRegistration: RegisteredComponent
) => (aType[KEY_COMPONENT] = aRegistration);

export const registerLayoutMapping = (
  aType: ComponentTypeRef,
  aRegistration: RegisteredLayoutMapping
) => (aType[KEY_LAYOUT_MAPPING] = aRegistration);

export const getRegisteredComponent = (
  aType: ComponentTypeRef
): RegisteredComponent => aType[KEY_COMPONENT];

After compilation the generated .d.ts file is:

import { AbstractRegisteredComponent, AbstractRegisteredLayoutMapping } from '@acoustic-content-sdk/component-utils';
import { ComponentTypeRef } from '@acoustic-content-sdk/react-api';
import { LayoutComponentDirective } from './layout.directive';
export declare type RegisteredComponent = AbstractRegisteredComponent<ComponentTypeRef, LayoutComponentDirective>;
export declare type RegisteredLayoutMapping = AbstractRegisteredLayoutMapping<ComponentTypeRef>;
export declare const registerComponent: (aType: import("@acoustic-content-sdk/react-api").ReactComponent<import("@acoustic-content-sdk/react-api").ReactComponentProps, any>, aRegistration: RegisteredComponent) => RegisteredComponent;
export declare const registerLayoutMapping: (aType: import("@acoustic-content-sdk/react-api").ReactComponent<import("@acoustic-content-sdk/react-api").ReactComponentProps, any>, aRegistration: RegisteredLayoutMapping) => RegisteredLayoutMapping;
export declare const getRegisteredComponent: (aType: import("@acoustic-content-sdk/react-api").ReactComponent<import("@acoustic-content-sdk/react-api").ReactComponentProps, any>) => RegisteredComponent;

Many (but not all) of the import statements have been inlined.

Why is this happening and how can I avoid this?
The underlying problem is that I am using @microsoft/api-extractor and @microsoft/api-documenter to generate documentation for my project. These tools rely on the information in the .d.ts files to generate their doc and with the inline imports this end-user facing documentation becomes very hard to read, if not unusable.

TypeScript Version: 3.7.4

Expected behavior:

I expect the import statements in the .d.ts files to match the import statements in the ts source.

Actual behavior:

Imports are sometimes inlined, sometimes not. I cannot tell what causes the imports to be inlined.

Related Issues:

#30258

@CarstenLeue
Copy link
Author

The difference to #30258 is that in that defect the inline import referred to a deduced type that had not been imported explicitly. The recommendation was If you add those imports to the file yourself, they'll be used, so I'd recommend doing that., I understood this as the hint to add explicit typing to the TS source and to import those types explicitly in the TS source.

This is exactly what's happening in this example, the type ComponentTypeRef has been imported explicitly, still it is inlined in some cases (for the aType variables) but not in other cased (for the declaration of the RegisteredComponent type).

@eps1lon
Copy link
Contributor

eps1lon commented Jan 24, 2020

We encountered this issue in mui/material-ui-pickers#1465.

The general gist is that /pickers has a peer dependency on /core. In development /pickers used /[email protected] when running tsc. In /pickers we had something like:

import { createStyles } from '@material-ui/core';

export const styles = createStyles();

where declare function createStyles(): StyleRules which emitted the following type for styles: import('@material-ui/core').CSSProperties | () => import('@material-ui/core').CSSProperties.

The issue is that createStyles returns StyleRules which got changed in /[email protected] to only contain CSSProperties. Therefore the type emited in /pickers could not be assigned to StyleRules even though the runtime is equivalent (because it's a peer).

The correct declaration should be ReturnType<typeof import('@material-ui/cire').createStyles>. TypeScript should not inline types of dependencies.

@RyanCavanaugh
Copy link
Member

In OP's example, TypeScript has three options:

  • Add a new import of ReactComponent to the import declaration at the top of the file
  • Refer to import("@acoustic-content-sdk/react-api").ReactComponentProps
  • Attempt to render a structural substitution of ReactComponentProps (which is very likely to fail in one way or another)

The first option has the problem that we could potentially introduce name conflicts, or need to do some name mangling to make sure existing other meanings of ReactComponent in that file were maintained.

The second option is always safe.

The third option is terrible and people hate it when we do this 🙂

You can add an import of ReactComponent to your extant import declaration if you don't want inline type imports to be added to your .d.ts, but in the general case, a local import is going to give the best behavior.

@RyanCavanaugh RyanCavanaugh added the Question An issue which isn't directly actionable in code label Feb 5, 2020
@CarstenLeue
Copy link
Author

@RyanCavanaugh I am not following this train of thought. The TS in question contains:

import { ComponentTypeRef } from '@acoustic-content-sdk/react-api';

I would expect this type to be used in the typings file. There is no need to add a new import as mentioned in (1) nor to reference ReactComponentProps directly as in (2).
What seems to happen is that the TS compiler denormalized the ComponentTypeRef, but why? I appears to me that this might even be problematic in scenarios that @eps1lon mentioned.

So I would prefer the fourth option, the TS compiler should use the type declaration as it was in the source (unless I am missing a point here).

@typescript-bot
Copy link
Collaborator

This issue has been marked as 'Question' and has seen no recent activity. It has been automatically closed for house-keeping purposes. If you're still waiting on a response, questions are usually better suited to stackoverflow.

@weeebdev
Copy link

@RyanCavanaugh I am not following this train of thought. The TS in question contains:

import { ComponentTypeRef } from '@acoustic-content-sdk/react-api';

I would expect this type to be used in the typings file. There is no need to add a new import as mentioned in (1) nor to reference ReactComponentProps directly as in (2).
What seems to happen is that the TS compiler denormalized the ComponentTypeRef, but why? I appears to me that this might even be problematic in scenarios that @eps1lon mentioned.

So I would prefer the fourth option, the TS compiler should use the type declaration as it was in the source (unless I am missing a point here).

Did you solve this? Any workarounds?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

5 participants