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

type export issue #23280

Closed
mjbvz opened this issue Apr 9, 2018 · 12 comments
Closed

type export issue #23280

mjbvz opened this issue Apr 9, 2018 · 12 comments
Labels
Duplicate An existing issue was already created

Comments

@mjbvz
Copy link
Contributor

mjbvz commented Apr 9, 2018

From @waitingsong on April 7, 2018 3:12

Issue Type: Bug

tsconfig.json

{
  "compilerOptions": {
    "alwaysStrict": true,           /* Parse in strict mode and emit "use strict" for each source file. */
    "declaration": true,            /* Generates corresponding '.d.ts' file. */
    "module": "commonjs",           /* 'commonjs', 'amd', 'system', 'umd' or 'es2015'. */
    "strict": true,                 /* Enable all strict type-checking options. */
    "target": "ES2017",              /* 'ES5', 'ES2015', 'ES2016', 'ES2017', or 'ESNEXT'. */
    "types" : ["node"]
  }
}

package.json

{
  "name": "",
  "version": "1.0.0",
  "description": "",
  "private": true,
  "scripts": {},
  "dependencies": {},
  "devDependencies": {
    "@types/node": "^7.0.12",
    "tslib": "^1.9.0",
    "typescript": "^2.8.1"
  },
  "engines": {
    "node": ">=8.9.0"
  }
} 

config/config.default.ts

export interface Config {
  keys: string
  news: {
    pageSize: number;
  };
}

export type PowerPartial<T> = {
  [U in keyof T]?: T[U] extends {}
  ? PowerPartial<T[U]>
  : T[U]
}

// for config.{env}.ts
export type DefaultConfig = PowerPartial<Config>;

config.local.ts

import { DefaultConfig } from './config.default';

export default () => {
  const config: DefaultConfig = {};

  config.sourceUrl = 'foo'
  config.news = {
    pageSize: 20,
  };

  return config;
}; 

got error:

config/config.local.ts(3,1): error TS4082: Default export of the module has or is using private name 'PowerPartial'.
config/config.local.ts(6,10): error TS2339: Property 'sourceUrl' does not exist on type 'PowerPartial<Config>'.
11:06:36 - Compilation complete. Watching for file changes.

if change to `"declaration": false,``` , then issue gone.
any suggestion ?

VS Code version: Code 1.21.1 (79b44aa704ce542d8ca4a3cc44cfca566e7720f1, 2018-03-14T14:46:47.128Z)
OS version: Windows_NT x64 6.1.7601

System Info
Item Value
CPUs Intel(R) Core(TM) i5 CPU 760 @ 2.80GHz (4 x 2809)
Memory (System) 15.97GB (4.32GB free)
Process Argv C:\Program Files\Microsoft VS Code\Code.exe E:\project\tt\ts\config\config.local.ts
Screen Reader no
VM 67%
Extensions: none Reproduces without extensions

Copied from original issue: microsoft/vscode#47374

@mjbvz mjbvz self-assigned this Apr 9, 2018
@mjbvz
Copy link
Contributor Author

mjbvz commented Apr 9, 2018

From @waitingsong on April 7, 2018 3:17

If change

export interface Config {
  keys: string
  news: {
    pageSize: number;
  };
}

without sub object news, then issue gone

export interface Config {
  keys: string
 // news: {
 //   pageSize: number;
 //  };
}

@mjbvz
Copy link
Contributor Author

mjbvz commented Apr 9, 2018

From @waitingsong on April 7, 2018 3:46

maybe bug of TypeScript ?

@mjbvz mjbvz removed the typescript label Apr 9, 2018
@mjbvz mjbvz removed their assignment Apr 9, 2018
@ghost
Copy link

ghost commented Apr 9, 2018

Simplified repro:
a.ts

export interface I { x: number; }
export type J = I;

b.ts

import { J } from './a';
export const f = (): J => ({ x: 0 });

Result:

src/b.ts(2,14): error TS4023: Exported variable 'f' has or is using name 'I' from external module "/home/andy/sample/ts/src/a" but cannot be named.

No error when using export function f(): J { return { x: 0 }; } `.

@weswigham May be related to recent declaration emit changes?

@ghost ghost added the Bug A bug in TypeScript label Apr 9, 2018
@weswigham
Copy link
Member

I don't think so; this is a visibility error, and the mechanics of those didn't change with the emitter rewrite; we're probably just not considering alias visibility when attempting to write I inside the type of f. Is this a regression? If so, I'd wager it'd be due to a change within getAccessibleSymbolChain.

@ghost
Copy link

ghost commented Apr 9, 2018

Nope, looks like this was a bug way back in typescript@2.0.

@weswigham
Copy link
Member

So probably just another longstanding bug in getAccessibleSymbolChain then 😝

@weswigham
Copy link
Member

weswigham commented Apr 9, 2018

Riiiiight, so this isn't strictly a bug but it's not strictly desired behavior either. Type aliases aren't currently considered aliases of something because they can do things like this:

// @declaration: true
// @filename: a.ts
export interface I<T> { x: T; }
export type J = I<any>;

// @filename: b.ts
import { J } from './a';
export const f = (): J => ({ x: 0 });

note how the type alias is an instantiation of the type. Another:

// @declaration: true
// @filename: a.ts
export interface I<T> { x: T; }
export type J<Q> = I<Q>;

// @filename: b.ts
import { J } from './a';
export const f = (): J<number> => ({ x: 0 });

in this case it forwards a type parameter.
It's only in a (maybe common?) subset of the allowed RHS where it is effectively an alias for some other type, such as in the OP. In the general case, a type alias may not be used as an alias for some other type, as it is a distinct entity with potentially differing behaviors/instantiations/constraints. If we wanted to make this work, what we'd effectively have to do is check if the type of a type alias is identical to the type we're looking for and if so, allow its use as a reference for that type (and even that is sometimes wrong, like if the alias is I | never - that's identical to I but probably shouldn't qualify as a real alias).

@mhegazy is this worth special-casing?

@weswigham
Copy link
Member

weswigham commented Apr 9, 2018

Hmmm... on the other hand, in both cases (the OP and @andy-ms 's example) there's explicitly a reference to the alias somewhere in the types the make up the expression - we probably aught to be writing the output type using that rather than what the alias resolves to anyway; this may just be a symptom of eager alias resolution, endemic of how we treat type aliases.

@mhegazy
Copy link
Contributor

mhegazy commented Apr 9, 2018

I would just say this is a duplicate of #9944. once #9944 is addressed, no error should be shown in this scenario.

@weswigham
Copy link
Member

weswigham commented Apr 10, 2018

@mhegazy This is honestly a little different. (Though that would incidentally fix the issue in the OP.) In this case, we're opting not to write out the type as the user wrote it (ie, with an alias) and we issue a visibility error because of it. And we write out the type that way because we forgot the alias ever existed. 🤷‍♀️

@mhegazy
Copy link
Contributor

mhegazy commented Apr 10, 2018

Maintaining the alias is a different issue. do not think we have a simple or clear fix for this one. so i would say that part is a design limitation.
Adding the import will address the biggest pain point in this scenario, yes it would be great if it was written the exact same way a human would have wrote it, but it will be close enough.

@typescript-bot
Copy link
Collaborator

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

@microsoft microsoft locked and limited conversation to collaborators Jul 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants