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 and module imports of the same name shouldn't fail with "duplicate identifier" #48764

Closed
samhh opened this issue Apr 19, 2022 · 13 comments
Closed
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@samhh
Copy link

samhh commented Apr 19, 2022

Bug Report

πŸ”Ž Search Terms

  • import type
  • duplicate identifier
  • namespace
  • type
  • value

πŸ•— Version & Regression Information

This is the behavior in every version I tried, and I reviewed the FAQ for entries about import type.

⏯ Playground Link

I don't think this is possible to demonstrate in the playground as it needs a module to import * from?

πŸ’» Code

// Works
import type { T } from 'foo'
declare const T: any

// Doesn't work, but should
import type { T } from 'foo'
import * as T from 'bar'

πŸ™ Actual behavior

Duplicate identifier 'T'.

πŸ™‚ Expected behavior

The compiler should recognise that one identifier is a type and the other a value and allow the two to coexist, as it does in other scenarios.

@RyanCavanaugh
Copy link
Member

This is the intended behavior; import type still brings along the value meaning (if there is one), as you can demonstrate by writing something like

export class Foo { }
import type { Foo } from "./other";
// Legal to use value meaning of Foo (in type contexts)
function fn(x: typeof Foo) { }

You're allowed to "shadow" import type with a const, but we don't allow you to have potentially-conflicting imports since there's no clear scoping rule to apply to that scenario.

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Apr 19, 2022
@samhh
Copy link
Author

samhh commented Apr 19, 2022

I didn't know that about import type!

Would it be possible to check what Foo is before bailing? In my use case Foo has no value reference so if I'm following correctly it should be unambiguous.

Specifically my use case is to import a module that represents a type and its operators as both a namespace (* as) and a type. This is a very common pattern in fp-ts, where you might import something like:

import * as Task from 'fp-ts/Task'

Ideally per this issue we could do:

import type { Task } from 'fp-ts/Task'

Right now at best you can redefine the type:

type Task<A> = Task.Task<A>

Or worse yet carry the redundant namespace around with you everywhere:

declare const x: Task.Task<A>

@RyanCavanaugh
Copy link
Member

In my use case Foo has no value reference so if I'm following correctly it should be unambiguous.

We don't want to create a forward-versioning hazard in the referenced module. In other words, adding a value meaning to an identifier in your module should not possibly break people who refer to that module.

@ritschwumm
Copy link

ritschwumm commented Apr 19, 2022

@RyanCavanaugh can you elaborate how adding a value meaning later could break existing code? is that only the case because import type implicitly brings along the value meaning? additionally i'd like to understand why it does that, at first glance that sounds counter-intuitive to me.

the reason i'm asking: i was looking for a solution to exactly the same problem @samh describes. right now i have two separate imports for half a dozen modules in almost every file, and it's starting to annoy me so much that i earnestly consider to rename all my types to T so i can at least do something like this:

import * as either from "@lib/either";

const foo:either.T<A,B> = either.left(...);

instead of

import { Either } from "@lib/either";
import * as either from "@lib/either";

const foo:Either<A,B> = either.left(...);

@RyanCavanaugh
Copy link
Member

Sure, let's say you write

// other.ts
export interface Foo { }
// another.ts
export const a = 0;
// myFile.ts
import type { Foo } from "./other";
import * as Foo from './another';

This is the thing that you want to be OK. But if we did, then if you updated other.ts

// other.ts
- export interface Foo { }
+ export class Foo { }

Now Foo is ambiguously the class value or the import * value and there's a new error in myFile.ts

@ritschwumm
Copy link

so if import type did not implicitly bring along the value Foo everything would be fine?
i'm trying to see how this behaviour is useful (apart from the fact that existing code of course relies on it).

@samhh
Copy link
Author

samhh commented Apr 20, 2022

Seconding @ritschwumm I've never seen import type used for anything other than importing actual type/interface declarations, the behaviour is surprising to me.

@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@ritschwumm
Copy link

it seems the meaning of import type <name> is not - as we both assumed - "import the named type", but "import everything about things with this name that does not actually make it necessary to provide access to the values of this name".

@samhh
Copy link
Author

samhh commented Apr 23, 2022

@RyanCavanaugh I'm unsure what to do regarding the closure of this ticket because this is still a pain point regardless of how it could potentially be solved. Should I raise a new ticket?

@scottwillmoore
Copy link

I agree, it would be great if there was a good solution to this problem. You cannot do this:

// Point.ts
export type Point = { x: number, y: number };
export add = (a: Point, b: Point): Point => ({ x: a.x + b.x, x: a.y + b.y });

// index.ts
import type ( Point } from "./Point";
import * as Point from "./Point";

const a: Point = { x: 2, y: 4 };
const b: Point = { x: 2, y: 4 };

const c = Point.add(a, b);

console.log(c); // { x: 4, y: 8 };

The only workaround that I have been able to find is this:

export type Point = { x: number, y: number };
export const Point = { add: (a: Point, b: Point): Point => ({ x: a.x + b.x, x: a.y + b.y }) };

// index.ts
import ( Point } from "./Point";

const a: Point = { x: 2, y: 4 };
const b: Point = { x: 2, y: 4 };

const c = Point.add(a, b);

console.log(c); // { x: 4, y: 8 };

However, this is ugly and also has some significant differences.

(1) You now cannot import a single function, and must import the entire object. I guess this could be fixed by exporting and function and an object that contains the function, as below:

// You cannot do this:
import ( Point, add } from "./Point";

// Unless you export twice:
export type Point = { x: number, y: number };
export add = (a: Point, b: Point): Point => ({ x: a.x + b.x, x: a.y + b.y });
export const Point = { add };

(2) You get the error 'Point' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer. when you reference Point in an initializer.

export type Point = { x: number; y: number };

export const Point = {
  zero: (): Point => ({ x: 0, y: 0 }),

  add: (a: Point, b: Point, c = Point.zero()): Point => {
    c.x = a.x + b.x;
    c.y = a.x + b.y;
    return c;
  },
};

(3) It can also cause problems when attempting to tree-shake the module. However, in fairness, most JavaScript optimizers are now able to identify and hoist the exported object.

TLDR

This pattern is very common in FP oriented libraries (gcanti/fp-ts#1003, gcanti/fp-ts#1044, gcanti/fp-ts#1415, gcanti/fp-ts#1633).

@cruhl
Copy link

cruhl commented Oct 25, 2022

@samhh @scottwillmoore Please help support the latest issue on this before it gets closed again πŸ˜‘
#51109

@cruhl
Copy link

cruhl commented Oct 25, 2022

Here is my ideal syntax...
import * as User, type { User } from "./User";

@RyanCavanaugh I don't think this should have been closed, even if it is "working as intended."
This issue has been creating friction for developers trying to write functional code for years now.
I've listed a bunch of relevant discussions in this related issue: #51109

It's bizarre that type/namespace/function merging is supported within a file, but not via imports and exports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

6 participants