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

Object detection for "Exported variable has or is using name from external module" #5938

Closed
Deathspike opened this issue Dec 4, 2015 · 16 comments · Fixed by #19852
Closed
Assignees
Labels
Bug A bug in TypeScript Domain: Declaration Emit The issue relates to the emission of d.ts files Fixed A PR has been merged for this issue

Comments

@Deathspike
Copy link

With the following structure:

  • node_modules/a/default.d.ts containing a typing and a function
  • node_modules/a/package.json containing a typings field to default.d.ts
  • src/default.ts' re-exporting everything from a and exporting new functions
  • src/newFunctions.ts using the src/default.ts

You get "exported variable has or is using name from external module" errors when using an object:

import * as b from './default';

// Bug: This does the same thing, but in an object. It should still compile fine, but it doesn't.
export let wrap = {
  findPersonById: function(id: number): b.IPerson {
    return b.findPersonById(id);
  }
}

However, when you don't use an object the functionality works as expected:

import * as b from './default';

// OK: Expected behavior. Since `b` exports everything from `a`, exporting `b.IPerson` is fine.
export function wrapFindPersonById(id: number): b.IPerson {
  return b.findPersonById(123);
}

Using 1.8.0-dev.20151204. Full repo https://github.com/Deathspike/typescript-reference-bug-example/

@mhegazy mhegazy self-assigned this Feb 19, 2016
@mhegazy mhegazy added this to the TypeScript 2.0 milestone Feb 19, 2016
@mhegazy mhegazy added the Bug A bug in TypeScript label Feb 19, 2016
@mhegazy mhegazy added Fixed A PR has been merged for this issue and removed Fixed A PR has been merged for this issue labels Mar 17, 2016
@mhegazy
Copy link
Contributor

mhegazy commented May 16, 2016

A simplified repro is available in #8612

@robyoder
Copy link

robyoder commented Dec 1, 2016

#8612 is what I'm trying to do. What is the difficulty of fixing this issue? Or, is there any way to work around this so that all the exports from a group of files can be imported into the same namespace?

@robyoder
Copy link

robyoder commented Dec 1, 2016

Aha. All I have to do is reference the original file, even if I don't use that reference. So, before, I had this:

////// model/index.ts

export { Team } from './Team'
export { Game } from './Game'
export { Score } from './Score'
////// action/index.ts

import * as model from '../model'

// This declaration produces the "cannot be named" error
export const getTeam = (teamId: string): model.Team => {
    // ...
}

But if I add import * as __Team from '../model/Team' to action/index.ts I'm error-free:

////// action/index.ts

import * as model from '../model'
import * as __Team from '../model/Team' // Throwaway reference

// No more error!
export const getTeam = (teamId: string): model.Team => {
    // ...
}

It's less than ideal, but at least I can keep my model.Team format.

@ymaxli
Copy link

ymaxli commented Feb 21, 2017

if setting compilerOptions.declaration to false, there will be no more error!

@robyoder
Copy link

robyoder commented Apr 6, 2017

Right... but that's not what we want. It should work fine whether we generate declaration files or not. In my case, I need the declaration files to be generated, and this workaround has proven successful, but quite the pain.

@alex-okrushko
Copy link

Any ETA on this? Thanks!

@atrauzzi
Copy link

Would love to see this one addressed, a bit pesky.

@axefrog
Copy link

axefrog commented Sep 16, 2017

At the very least, as a shorter-term solution, it'd be nice to see a compiler option that allows definition files to be generated, with definitions that "cannot be named" simply being omitted, or replaced with any. I want definition files, but I find it annoying that one my two co-dependent projects keeps insisting that the definitions in the other can't be named, like some kind of low-ranking bureaucrat who refuses to think beyond the literal wording in his employee manual.

mattmazzola added a commit to microsoft/ConversationLearner-UI that referenced this issue Sep 21, 2017
First step in getting more of the TypeScript compiler checks on to help prevent bugs. This flag is more aesthetics than behavioral; however, it does help debugging and navigating code when looking up references.  Since so many of the models and actions were imported into classes and not used it made it appear that they were used by more classes.

I did see a few issues around using Redux `connect` and microsoft/TypeScript#5938
It's related to trying to export Props which contains types from the Models class that were not explicitly imported.
I believe this is because we are using the `returntypeof` instead of explicitly defining the interfaces.

I noticed `connect` has generic overload <StateProps, DispatchProps, OwnProps> and it internally merges them all which is what I think we should be doing; however, currently don't have well established pattern so I left code as is.
mattmazzola added a commit to microsoft/ConversationLearner-UI that referenced this issue Sep 21, 2017
First step in getting more of the TypeScript compiler checks on to help prevent bugs. This flag is more aesthetics than behavioral; however, it does help debugging and navigating code when looking up references.  Since so many of the models and actions were imported into classes and not used it made it appear that they were used by more classes.

I did see a few issues around using Redux `connect` and microsoft/TypeScript#5938
It's related to trying to export Props which contains types from the Models class that were not explicitly imported.
I believe this is because we are using the `returntypeof` instead of explicitly defining the interfaces.

I noticed `connect` has generic overload <StateProps, DispatchProps, OwnProps> and it internally merges them all which is what I think we should be doing; however, currently don't have well established pattern so I left code as is.
@baio
Copy link

baio commented Oct 9, 2017

I've just started to move shared code from your /shared project folder to separate module and stuck with this issue. Very sad !

@robyoder
Copy link

robyoder commented Oct 9, 2017

In case you missed it, there is a workaround to this issue if you're stuck. I posted it above.

@theigor
Copy link

theigor commented Oct 21, 2017

@robyoder - I'm running into this same issue but with a module that's not mine. Specifically - express.

import * as express from "express";

let router = express.Router();

router.use(...);

export = router;

recreates this issue for router but I can't simply say import * as Router from "express/Router" because that doesn't exist. Is there a different workaround for this case or am I doing something wrong?

Thanks!

@theigor
Copy link

theigor commented Oct 21, 2017

Oh and I found a solution here - #9944 (comment)

@robyoder
Copy link

@theigor right, that's the solution. You could still use it as express.Router() though. All TS needs is the name to be imported. So you'd just add

import { Router } from "express";

but you don't actually need to change any other code (to use Router directly).

@mhegazy mhegazy added the Domain: Declaration Emit The issue relates to the emission of d.ts files label Oct 23, 2017
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Nov 9, 2017
@mhegazy mhegazy modified the milestones: Future, TypeScript 2.7 Nov 9, 2017
@robyoder
Copy link

@weswigham your fix is good for explicit re-exports like this:

export { Team } from "./Team";

but TS still fails when using a star export like this:

export * from "./Team";

I've pushed a simple project showing this issue: https://github.com/robyoder/tsctest

@weswigham
Copy link
Member

@robyoder Wanna open a new issue with the new repro? The reason for that failure is different from the underlying reason the named reexports were failing.

@robyoder
Copy link

@weswigham done: #20657

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

Successfully merging a pull request may close this issue.

10 participants