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

Spurious "Identifier 'angular' must be imported from a module" error #10638

Closed
jonrimmer opened this issue Aug 31, 2016 · 17 comments
Closed

Spurious "Identifier 'angular' must be imported from a module" error #10638

jonrimmer opened this issue Aug 31, 2016 · 17 comments
Assignees
Labels
Fixed A PR has been merged for this issue Suggestion An idea for TypeScript

Comments

@jonrimmer
Copy link

TypeScript Version:

2.0.2

Code

https://github.com/jonrimmer/typescript-bug

npm install
./node_modules/.bin/tsc bug.ts

Expected behavior:

It should compile without problems.

Actual behavior:

It gives the following error:

node_modules/angular-ui-router/commonjs/ng1/services.d.ts(2,27): error TS2686: Identifier 'angular' must be imported from a module

This is despite the fact that the @type/angular definitions includes a global angular var declaration.

@Gugic
Copy link

Gugic commented Aug 31, 2016

I have the same problems (trying to switch from typings to @types in my project)

To be more clear:
I just change my typings setup (angular, jquery, lodash, angular-ui-router) with same @types and

    "typeRoots": ["node_modules/@types"],
    "types": ["lodash", "jquery", "angular", "angular-ui-router"]

in my tsconfig.json and starting to get dozens of "Identifier 'angular' must be imported from a module" errors during my webpack build.

Typescript 2.0 and 2.0.2, webpack 2.1-beta.21+awesome-typesript-loader.

@mhegazy
Copy link
Contributor

mhegazy commented Aug 31, 2016

this is an issue in angular-ui-router declaration file. The compiler tries to ensure that a UMD module is used consistently, either as a module or as a global.

File: node_modules\angular-ui-router\commonjs\ng1\services.d.ts

should be:

import * as angular from "angular";
import IInjectorService = angular.auto.IInjectorService;

@mhegazy mhegazy added the External Relates to another program, environment, or user action which we cannot control. label Aug 31, 2016
@chrisnicola
Copy link

I don't think this is an angular-ui-router issue. I have the same problem without that installed coming from any local file that references angular.

Adding import * as angular from "angular" to every file is obviously one way to fix this, but this was not a problem before when I was using typings instead of @types. If this is just how the new type definition resolution works now then that is pretty disappointing.

@chrisnicola
Copy link

Worth noting that only angular seems to have this problem, jQuery, Lodash and others are working fine.

@mhegazy
Copy link
Contributor

mhegazy commented Sep 14, 2016

the rational here, is some libraries are available as either module or a global. if you are using them as a module, all references should be done this way. the warning aims to notify you about this discrepancy. this is a new feature added in TS 2.0 see https://github.com/Microsoft/TypeScript/wiki/What%27s-new-in-TypeScript#support-for-umd-module-definitions for more details.

@chrisnicola
Copy link

I have to be honest, while I basically understand the reasoning, the choice of making this work globally only if there are no imports or exports in the file seems completely arbitrary and comes across as a bad choice for both usability and backwards compatibility. At the very least this should have been configurable or provide an easy way to work around it.

I am now spending a great deal of what feels like unnecessary time on this while trying to move a fairly large project towards Angular 2 and Typescript 2 and this is creating unnecessary friction. If every update to the Typescript project is going to just add a new bar to jump over like this I'm really going to need to reconsider using it.

So far I've found this to be a common pattern with using Typescript. Many inexplicable behaviours and decisions each of which is hard to track down or find documentation around.

@RyanCavanaugh
Copy link
Member

Considering removing the restriction on UMD globals to only apply to values. There's not much point to enforce uniformity in type/namespace usage.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript In Discussion Not yet reached consensus and removed External Relates to another program, environment, or user action which we cannot control. labels Sep 14, 2016
RyanCavanaugh added a commit to RyanCavanaugh/TypeScript that referenced this issue Sep 14, 2016
RyanCavanaugh added a commit to RyanCavanaugh/TypeScript that referenced this issue Sep 14, 2016
@RyanCavanaugh RyanCavanaugh self-assigned this Sep 14, 2016
RyanCavanaugh added a commit to RyanCavanaugh/TypeScript that referenced this issue Sep 14, 2016
@mhegazy mhegazy added Fixed A PR has been merged for this issue and removed In Discussion Not yet reached consensus labels Sep 15, 2016
@mhegazy mhegazy added this to the TypeScript 2.0.3 milestone Sep 15, 2016
@chrisnicola
Copy link

chrisnicola commented Sep 15, 2016

Thanks @RyanCavanaugh it is probably also worth noting that calling import * as angular from 'angular' in each file does not work properly with Webpack as it will try to load angular multiple times.

The only solution is to add declare var angular at the top of each problematic file, which is far less than ideal.

@RyanCavanaugh
Copy link
Member

... does not work properly with Webpack as it will try to load angular multiple times.

How does anyone use Webpack with modules, ever, if this is its behavior? Can you show a small example of how repeated imports would lead to a problem?

@chrisnicola
Copy link

It's a fair question, there may be issues currently with Webpack 2 changes working with the TS loaders actually. I've found both ts-loader and awesome-typescript-loader can behave inconsistently with respect to just running tsc

Also the behaviour may have been specific to running on the webpack-dev-server. I didn't spend a ton of time testing it.

I'll see if I an produce a small example when I have a chance.

kakaruna added a commit to kakaruna/TypeScript that referenced this issue Sep 26, 2016
* Factor out getRenameInfo

* Update Jakefile

* Correct emit comment for decorated class declaration

* Add tests and baselines

* Allow -Infinity as an enum property name

* Move allocators.ts to services.ts, meaning.ts to utilities.ts, and transpile functions to a new file transpile.ts

* Correct strings based on linting rules

* Force source-map-support to not have source maps

It fooled sorcery's incorrect check for sourceMappingURL into thinking
it had a source map.

Also up the error stack trace limit to 1000 to help future error
reporting.

* Only emit comment only once in module declaration with identifier path name

* Only emit comment once for export enum declaration

* When emitting react code, replace HTML numeric entities with their encoded characters

* Also decode entities when emitting attributes. Also, lexer should not process string escapes in jsx attributes.

* Use undefined, not null, to satisfy linter

* Update preferConstRule to use getCombined*X*Flags

now that they are exported.

* Remove another unused function in preferConstRule

* Yep, another unused function in preferConstRule

* Revert "Remove failing fourslash tests (may need to be restored and fixed)"

This reverts commit 2a60f79.

* Revert "Remove failing fourslash tests (may need to be restored and fixed)"

This reverts commit 2f9c9c9.

* Number is assignable to enum, even inside union

Previously, numbers were not assignable to enums that were inside a
union

* Test that number is assignable to enum in a union

* Use baselines for quick info tests to ease updates

* Fix duplicate baselines

* Use shorthand properties

* A shorthand ambient module should be considered as possibly exporting a value.

* Enum literal is assignable to enum, even inside union

Previously, only number literals were assignable to enums inside unions.

* Test that enum literal is assignable to an enum in a union

* Fix tests

* Accept baselines after merge

* Fix fourslash test

* Make preProcessFile public again

* Fix both new enum assignability predicates

And update error reporting baseline (new error is less elaborate)

* Fix classifier as well.

* Fix deferred export of array binding pattern

* Move code to a new module documentRegistry.ts

* Handle `OmittedExpression` nodes in binding patterns

* Surfacing method to get Completion Symbol instead of details for better extensibility

* Surfacing function in services.ts

* Assume outer variables are always initialized in control flow analysis

* Add regression tests

* Don't try to unlink folders

* Fix line endings

* Fix {Map,WeakMap}.prototype.set method signatures (microsoft#10694)

* Document `endOfChain`

* Adding method stub for the LanguageServiceShimProxy implementation

* method stub throwing an error for SessionClient

* lint: remove trailing whitespace in completions.ts

* Simplify parameters of `quickInfoIs`: `expectedtext` must be present and `expectedDocumentation` must be a string or ommitted, never null.

* minor changes from PR feedback

* Code cleanup and a few edge cases

* Disallow comma operator when LHS is pure

* Prevent duplicate entries from type references

* Added a STRATEGY placeholder for the --moduleResolution option

* Preserve type parameter types in narrowing

* Add regression test

* Fix ECMA-402 declarations (issue microsoft#10618)

1. Make String.prototype.localeCompare's `locales` parameter optional,
   so `undefined` is allowed.
2. Declare the `locales` parameter as a `string | string[]` union
   instead of using overloads. Having separate overloads for `string`
   and `string[]` unnecessarily prevents passing a `string | string[]`.
   (These overloads predate the introduction of union types.)

* Accept new baselines

* Handle const binding elements with initializers correctly

* Add tests

* Update tests

* Address PR comments

1. Cache results of isEnumTypeRelatedTo
2. Make numeric literal assignment stricter again.
3. Use isEnumRelatedTo for comparing enums to each other. This provides
the previous semi-structural semantics.
4. Because of the new distinction between computed enums (no union
members) and union enums (no computed values => a union of enum
literals), some semi-structural code moves out to the body of
`isRelatedTo`.

* More tests of enum assignability

1. Numeric literal <-> enum literal assignability
2. Computed enum <-> union enum assignability
3. Also rebaseline error reporting of existing enum cases.

* Fix lint

* Update baselines

* PR feedback

* Fix issue with helper emit.

Fixes microsoft#10800

* Quick bail out when narrowing type any by equality

* Add regression test

* Support export default for target=ES5/module=ES6.

Fixes microsoft#10855

* Update failing test baseline

* Enum assignability:loosen numbers+tighten computed

1. All numbers and numeric literals are assignable to all enums and enum
literals.
2. Computed enums are no longer assignable to anything except
themselves, even if they would otherwise be "semi-structurally"
compatible.

* Update baselines for updated enum assignability

* Fix missing asteriskToken for target=es6/module=amd.

Fixes microsoft#10857.

* Computed enum assignability is semi-structural

* Update baselines

* Disallow left comma operator operands which don't have side effects

* No widening of inferred type when initializer has a type assertion

* Accept new baselines

* Add test

* Fix missing final label.

Fixes microsoft#10876

* Implement NavigateTo for single files, instead of the project.

* Get rid of BOM

* Consume exceptions when checking for import completions

* Also wrap getEffectiveTypeRoots in import completion code

* Added test for scenario.

* Fix captured block scope variables in downlevel async.

Fixes microsoft#10889

* Remove unnecessary parentheses

* PR feedback and clean up.

* Add stackTraceLimit; update harness/tsconfig.json

1. Add stackTraceLimit argument to runtests.
2. Copy missing compiler files from compiler/tsconfig.json to
harness/tsconfig.json

* Simplify quick-info tests

* Flip check, add SEF cases

* Baseline update

* Add JSX to SEF exprs

* Fix build tasks for iocapture

* Added comments for __generator, reduced overall size of helper

* Add Code of Conduct

* Removed Code of Conduct from contributing, since readme is more obvious

* Fix build error caused by merge

* move to contribute header

* More PR feedback

* Use diffrent error message for namespaces unexported members

* Update other defintions of findIndex

* Rename function and use `directorySeparator` variables

* Remove duplicate test

* Lint

* Fix issue 10843

* add test for issue 10843

* Match number and string literal types to number and string in inference

* Add readonly typings for Set and Map

Similar to ReadonlyArray, these typings remove the mutation methods from Set and Map so that they can only be initialized by the constructor.

* Accept new baselines

* Add regression test

* Fix test issues due to an out of order merge

* Fix super in down-level async method

* add release-2.0

* More PR feedback

* Address CR feedback

* Allow type and NS references to UMD globals from modules

Fixes microsoft#10638

* Add comment explaining a magic constant in parser

* Fixing formatting

* revert tests

* revert fix for generated files

* update findIndex for es5 typed arrays

* Make declaration emit test name consistent

* Update baselines

* Removed constructor typings which can't be used

Also corrected some parameter names.

* indenting

* Add tests

* Serialize type alias when type alias symbol is not accessible

* Update baselines

* Address PR

* Fix build break: difference result from treating things as literal type

* Address PR: Update comment

* change error message for assigning from object

* Fix Reflect has method signature(s) per issue microsoft#10949 initial report

* fix linting error

* Output the help message line by line.

* Pass the jake test

* Handle msising tags for JsDoc nodes

* Add back getSourceFile and mark it as deprecated

* Use TypeFlags.Never to check for 'never' type

* Use silent never type in control flow loop analysis

* Add regression test

* Accept new baselines

* Take a few changes.

* fix typo

* remove extra files

* Correctly remove stale .error.txt baselines

* Fix downlevel async hoisting

* Fix some issues with module ES6/target ES5

* Fix missing visit of expression in for..of
@evmar
Copy link
Contributor

evmar commented Dec 22, 2016

@mhegazy @RyanCavanaugh @rkirov We have ~3k source files across multiple projects that rely on the old behavior. Do you have any advice on how to update to the new typings? I'm happy to update to the new module-like behavior but I was hoping to make some sort of temporary shim (that makes the new "angular" module also available as a global) so I could migrate a few files a time. It seems this error is accidentally designed to prevent us from that(?).

@mhegazy
Copy link
Contributor

mhegazy commented Dec 22, 2016

#10178 tracks relaxing this check.

@kenisteward
Copy link

@chrisnicola Webpack has a loader called CommonChunksPlugin that checks to see what modules have already been loaded to be sure not to have 2 in the bundle.

@DanielJoyce
Copy link

Yet another day wasted trying to make TSC happy. Yes yes, we all should modules, but not every library out there is compliant or nice. So this nonsense has got to stop.

@DanielJoyce
Copy link

@mhegazy Wow, that is a DUMB idea. I can't believe they are doing that, especially when trying to support legacy code bases.

@Gugic
Copy link

Gugic commented Feb 7, 2017

By the way, the title problem was solved by me in that way:

// _shim.d.ts
import * as __angular from "angular";

declare global {
    const angular: typeof __angular;
}

@DanielJoyce
Copy link

DanielJoyce commented Feb 7, 2017

@Gugic Apparently my globals file can't declare a globals namespace because its not ambient though it ends in .d.ts due to something else silently triggering it not being ambient or something

Augmentations for the global scope can only be directly nested in external modules or ambient module declarations.at line 64 col 9

There are so many pitfalls in ts modules

My globals.d.ts now consists entirely of

declare global {
}

and I still get that error

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Fixed A PR has been merged for this issue Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

8 participants