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

Enum used as object key is not exported in declaration #35329

Open
hcomnetworkers opened this issue Nov 25, 2019 · 3 comments
Open

Enum used as object key is not exported in declaration #35329

hcomnetworkers opened this issue Nov 25, 2019 · 3 comments
Labels
Bug A bug in TypeScript Domain: Declaration Emit The issue relates to the emission of d.ts files
Milestone

Comments

@hcomnetworkers
Copy link

hcomnetworkers commented Nov 25, 2019

TypeScript Version: 3.8.0-dev.20191125 (@next), but was present in 3.4 already

Search Terms:

  • typescript ts4023
  • Exported variable has or is using name from external module but cannot be named
  • typescript enum declaration

Code

Selectors.ts

import {State} from './Store';
export const pageUiSelector = (state: State) => state.ui;

Store.ts

export enum Pages {
  dashboard = 'dashboard',
}
export interface State {
  ui: {
    [Pages.dashboard]: {
      selectedRow: number | null;
    };
  };
}

tsconfig.json

{
  "compilerOptions": {
    "baseUrl": ".",
    "declaration": true,
    "module": "commonjs",
    "rootDir": ".",
    "strict": true,
    "target": "es2017"
  }
}

Expected behavior:
No error.

Actual behavior:
This error is thrown:

Selectors.ts:2:14 - error TS4023: Exported variable 'pageUiSelector' has or is using name 'Pages' from external module "xxx/Store" but cannot be named.

Details:
This error only occurs when exporting the given selector and when using --declaration. The probable cause is that the enum values used as object key is not properly detected and added to the exported files.

If the enum is not used as object key, but as object value, the error does not occur.
If the content of Store.ts is moved to Selectors.ts, the error does not occur.

This code works as expected in the ts-loader via webpack, however tsc and PhpStorm report this as error.

@andrewbranch
Copy link
Member

The cause is that the type serializer can’t figure out what to write for the return type of pageUiSelector’s function signature, presumably because an inline import type isn’t valid in a property key position:

image

I’m not sure if there’s a reason we can’t fall back to using an indexed access type, State["ui"]. Also, it seems like if you had imported Pages into Selectors.ts, we should be able to write the type since the import type wouldn’t be necessary, but you still get the same error. I think that’s a bug, not 100% sure whether your exact repro is or not. @weswigham thoughts?

Anyway, the cleanest workaround is to give State["ui"] its own named exported type:

export enum Pages {
  dashboard = 'dashboard',
}

export interface UIState {
  [Pages.dashboard]: {
    selectedRow: number | null;
  };
}

export interface State {
  ui: UIState;
}

@andrewbranch andrewbranch added the Needs Investigation This issue needs a team member to investigate its status. label Dec 19, 2019
@weswigham
Copy link
Member

weswigham commented Dec 19, 2019

Hurrrr - it's because we allow A.B.C (an entity name) in a computed name position, and we compute import("a").B.C as a kind of extended entity name, but that kind isn't allowed as a computed property name (because computed names are a strange intersection of type expressions and value expressions).

To fix, on our part, we could either

  • Issue an error that Pages is inaccessible and request an explicit return type (this is inline with current logic and is most idiomatic), or
  • Add import {Pages} from "./Store" to the declaration file ourselves, and write as Pages.dashboard (which we've talked about as being a desirable direction for declaration emit in the past), or
  • Retain knowledge that state.ui is an alias to State["ui"] and write out like that (which would necessitate recording kinds of "aliasing types" (versus the alias symbols we track today) like I've implemented in the past), or
  • Split parsing of ComputedPropertyName to use different constructs between type contexts and value contexts. In type contexts, the above could easily be considered valid (as it's just an import type reference). This would mean parsing an Expression in value contexts, and a TypeNode in type space. That would also allow complex type things to be inlined, like {[Calc<"field">]: number}.

@hcomnetworkers As a workaround, add an import to Pages in-scope, and we should use that in declaration emit, rather than the inline import type, or explicitly annotate the return type of the lambda function.

@weswigham weswigham added Bug A bug in TypeScript Domain: Declaration Emit The issue relates to the emission of d.ts files and removed Needs Investigation This issue needs a team member to investigate its status. labels Dec 19, 2019
@andrewbranch
Copy link
Member

@weswigham sorry, I think my comment misled you. The screenshot I posted is something I wrote for the purpose of illustrating that import types aren‘t valid property key types. That didn’t come from the declaration emit.

Issue an error that Pages is inaccessible and request an explicit return type

This is what’s happening today. I was unsure if it’s a bug.

As a workaround, add an import to Pages in-scope, and we should use that in declaration emit

This is not a functioning workaround, which I did think is a bug:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: Declaration Emit The issue relates to the emission of d.ts files
Projects
None yet
Development

No branches or pull requests

4 participants