Skip to content

ERROR in [default] \node_modules\angular2-logger\app\core\logger.ts:62:38 Type 'string' is not assignable to type 'Level'. #65

@vkniazeu

Description

@vkniazeu

This problem appeared with angular2-logger 0.3.0 after I updated to Angular 2 rc5.
I just updated angular2-logger to its latest 0.4.2 and the problem still shows up.

ERROR in [default] \node_modules\angular2-logger\app\core\logger.ts:62:38 Type 'string' is not assignable to type 'Level'.

Changing the line from:

private _loadLevel = (): Level => localStorage.getItem( this._storeAs );

to

private _loadLevel = (): any=> localStorage.getItem( this._storeAs );

seems to resolve the problem.

Activity

langley-agm

langley-agm commented on Aug 26, 2016

@langley-agm
Contributor

Hi @vkniazeu ,

This issue appeared in a Typescript upgrade, not angular2-logger upgrade. See #39. If you use the latest officially released version of Typescript 1.8.10 you won't see it.

It should be fixed in 0.4.2 even for newer versions of Typescript though, if you are following the Quickstart guide. Somehow you are pointing to the logger.ts when you should only be using the typings ( core.d.ts ) as the library is already compiled, you don't have to compile it again.

Is there a step you are doing different from the Quickstart guide?

vkniazeu

vkniazeu commented on Aug 26, 2016

@vkniazeu
Author

Hi @langley-agm,
Thank you for such a quick reply and for a great logging utility!
I apologize I haven't found the original issue before opening this one and for the fact that I've been updating various aspects of my app, so I assumed a wrong thing.
I am using "angular-cli": "1.0.0-beta.11-webpack.2", which was recently ported over from SystemJS to webpack, which, I admit, is a much welcomed change and improvement IMHO.
That version of angular-cli requires Typescript 2.0.0, but I completely understand it's not an official release yet.
Step 2 of the Quickstart guide does not apply to webpack.
The way it handles typings under the angular-cli setup, is via the following in tsconfig.json:

    "typeRoots": [
      "../node_modules/@types"
    ],

I am not referencing or pointing to logger.ts explicitly in any way.
Just importing and listing the relevant classes in the providers array of the main app's module:

import { LOG_LOGGER_PROVIDERS, Logger } from 'angular2-logger/core';
...
  providers: [
    Logger, LOG_LOGGER_PROVIDERS,
  ]
...

Thank you for your help and insight!
If this issue is a duplicate and not related to angular2-logger, please close it.

langley-agm

langley-agm commented on Aug 26, 2016

@langley-agm
Contributor

Thanks for the kind words @vkniazeu.

Sadly I haven't been able to dig into webpack, there's just no way to sanely keep up with so many different frameworks poping up by the day and Angular Team not making up their minds about them doesn't make it any easier.

The root cause of this issue is the fact that Typescript is trying to compile a third party library in the first place. I don't think there's any other language that does that. The library is compiled in 1.8.10 and people should be able to use it in newer versions if they want to without finding these kind of conflicts. You already have the compiled files in the distribution, source files are merely for reference. That's why I don't want to just apply a change like the one you suggested because it will be hiding the real problem.

The Quickstart is based off the Angular 2's Quickstart which uses SystemJS so I applied a fix for it.

I'm thinking this is an issue on Webpack configuration but I haven't used it yet so I can't confirm it, if someone finds something missing in this repo that webpack needs in order to use the compiled files and not the source files I'll hapily look into it.

How are you telling webpack where to find 'angular2-logger/core' ? Is there a way to tell him to use either the .d.ts or even the js files instead of the ts files ?

One more thing I can think of that might be affecting / could help with this is the main and typings attribute in this library's package.json file.

vkniazeu

vkniazeu commented on Aug 27, 2016

@vkniazeu
Author

@langley-agm, I am fairly new to Typescript and brand new to webpack, but my understanding is that it goes through all .ts files referenced in your main.ts file and its children and uses the import statements to to bundle up any and all references withing your code.
There's no explicit webpack configuration within your project. Some people actually complain about the fact that the config is part of angluar-cli/webpack bundle and cannot be controlled manually, which imposes some restrictions.
Essentially, the only relevant file under your control is tsconfig.json, which is as simple as the following:

{
  "compilerOptions": {
    "declaration": false,
    "emitDecoratorMetadata": true,
    "experimentalDecorators": true,
    "lib": ["es6", "dom"],
    "mapRoot": "./",
    "module": "es6",
    "moduleResolution": "node",
    "outDir": "../dist/out-tsc",
    "sourceMap": true,
    "target": "es5",
    "typeRoots": [
      "../node_modules/@types"
    ],
    "types": [
      "jasmine"
    ]
  }
}

Many other packages like lodash, protractor, etc. seem to be putting their typings under node_modules\@types. Other packages, like angularfire2 seem to have updated their structure recently where external typings config is no longer required. They host only .js and .d.ts files within the package, not the source .ts.files.

On a separate note, aside from all the configuration and typings, wouldn't any strong type language compiler complain about the fact that your function is marked as returning a number (Level) while it actually returns a string in this line without any proper casting or a similar conversion?

private _loadLevel = (): Level => localStorage.getItem( this._storeAs );
langley-agm

langley-agm commented on Aug 27, 2016

@langley-agm
Contributor

Many other packages like lodash, protractor, etc. seem to be putting their typings under node_modules@types.

@types is a corporate name in NPM which holds different projects: https://www.npmjs.com/~types, you don't put your definitions there, you create a project there with the definitions, however this is useful for libraries that are not typescript.

If you notice lodash, protractor are js libraries not Typescript libraries, that's why you need the typings from a TS library which has no code but the definitions ( .d.ts files ), that's what typings mean. When you use these "libraries", you tell Typescript what's ok and what's not, without actually having the ts code of the js library, cause there's none. If your project is written in Typescript, it makes little sense to create a whole different project for the definitions only, that's why you import Angular 2 code from @angular and not @types, @angular is the corporate namespace in npm for the Angular Team projects going forward, this was done recently to avoid people from different companies not being able to name their projects with the same name. Another example is https://www.npmjs.com/package/%2540reactivex%2Frxjs.

Other packages, like angularfire2 seem to have updated their structure recently where external typings config is no longer required. They host only .js and .d.ts files within the package, not the source .ts.files.

Angular Team seems to be doing this, I don't know if just because it makes things easier for them or what, but they used to include them, RxJS does include them ( https://github.com/ReactiveX/rxjs ) and I definitely see a use case on including them, its been an innumerable number of times in which I've had to read Angular 2 source code and I see no reason why I should go to the Github and search for the code manualy when you can just have it there in your project and your IDE point you to them. Whether you download the source code or not should be a user's desicion imho, that's how other package managers work.

On a separate note, aside from all the configuration and typings, wouldn't any strong type language compiler complain about the fact that your function is marked as returning a number (Level) while it actually returns a string in this line without any proper casting or a similar conversion?

Well this is a decision on the language's creator/s. In Typescript 1.8.10 this isn't necessary, in newer versions it is. If I move my project to a newer version of Typescript I'll make sure to update that file. However, the typescript version in which this library is compiled, should not affect the project in which you use it, cause it is already compiled into JS. Again, whether this is correct or not, doesn't matter, the root cause of this issue is the fact that your project is trying to compile Third Party .ts files, you should not be forced to do this.

langley-agm

langley-agm commented on Aug 27, 2016

@langley-agm
Contributor

What happens if you try this? :

"typeRoots": [
    "../node_modules/@types",
    "../node_modules/angular2-logger"
],
vkniazeu

vkniazeu commented on Aug 27, 2016

@vkniazeu
Author

The latter suggestion didn't help, but it's okay.
I owe you not one, but a case of beer, for such an educational explanation! :)
To your point of having to go to Github to see the source, I get it and by no means I'm arguing, but shouldn't the JS (complied code) and the type definitions (= public interface to the package) normally be enough to use a module? I mean, if it weren't open source, that's all you'd get, right - just the publicly exposed interfaces?
Again, not arguing at all, but the technically the source code of a third-party module can be looked at something that should only live in their repo and not in the actual package that gets pulled to all the devs' machines.
Perhaps I'm misunderstanding something, but I hope I don't catch myself setting break points in the third-party modules to debug their Typescript implementation too often :))

langley-agm

langley-agm commented on Aug 27, 2016

@langley-agm
Contributor

shouldn't the JS (complied code) and the type definitions (= public interface to the package) normally be enough to use a module?

enough? yes, I'm not trying to go for what's the minimal enough though, if its useful for some users to have the source files there i'll leave them there. Like I mentioned other package managers have a flag to decide whether to include the source files in the downloaded packaged library or not, this is a lacking functionality in npm, however this is nowhere near the scope of this issue.

langley-agm

langley-agm commented on Aug 27, 2016

@langley-agm
Contributor

@vkniazeu If you are able to share your repo I can take a look at it, otherwise you could probably reproduce it in a small project that I do have access to. Not sure how hard it would be to do it in something like plunkr, probably its just easier to create a demo project in github because of all the dependencies and setup required in angular 2.

vkniazeu

vkniazeu commented on Aug 27, 2016

@vkniazeu
Author

@langley-agm, here's a minimal CLI webpack setup to reproduce the error.
It's just the output of the bare-bones setup listed at https://github.com/angular/angular-cli, with angular2-logger added to package.json and to app.module.ts.

npm install -g angular-cli
ng new test-cli
cd test-cli
ng serve

Download: test-cli.zip

vkniazeu

vkniazeu commented on Aug 28, 2016

@vkniazeu
Author

Reproducible with "angular2-logger": "0.4.5".

langley-agm

langley-agm commented on Aug 28, 2016

@langley-agm
Contributor

Yea, no real changes in these minor releases yet, I was just trying to fix some stuff rendering wrong from the .md file. It looks fine now in GitHub but npm still doing weird stuff with it.

TheFirstDeity

TheFirstDeity commented on Sep 9, 2016

@TheFirstDeity

The test setup posted by @vkniazeu will actually install the outdated SystemJS version of the cli. A better setup is the following:

npm install -g angular-cli@webpack
ng new test-cli -lc
cd test-cli
ng serve

Also note the -lc (aka --link-cli) switch which links the new project to the globally installed CLI instead of creating a local duplicate. Linking is probably a good idea in most cases.

Edit: Oops, disregard the next two paragraphs. Obviously we want to use the ES6 dist for CLI production builds because tree-shaking algorithms (removing unused code paths) work best with ES6 modules.

You can load an arbitrary script by putting it in the scripts section of your angular-cli.json file. For example,
"scripts": [ "../lib/angular2-logger.sys.min.js" ],
However, my angular-cli project was not able to load modules from it (I added the definitions in a declare module 'angular2-logger' { ... } block in my typings.d.ts and erased the node_modules/angular2-logger folder.) It might not support SystemJS's module format, and there are currently no hooks into the built-in webpack config to customize the build process (although it's a planned feature.)

One thing I haven't tried yet, though, is enabling "compileJs" in the typescript config and then referencing the bundled module with a relative path. Maybe it needs to be processed by the Typescript engine for the CLI's webpack build to pick it up properly?

Solution (Hack)

  1. Copy core.d.ts from node_modules/angular2-logger into node_modules/angular2-logger/dist/es6
  2. Use the es6 dist path in your import statements. Here are two examples:
    • import { Logger } from 'angular2-logger/dist/es6/core';
    • import * as logger from 'angular2-logger/dist/es6/core';

Conclusion

The angular CLI won't automatically pick up the es6 dist & core.d.ts, it needs both to be in the same directory and be pointed directly at it. If you have a look through node_modules\@angular\core (or any of them) you'll notice there are no .ts source files anymore, they were removed. Maybe that's just how Angular libraries are supposed to be structured? Or maybe there's a setting to configure it...

TheFirstDeity

TheFirstDeity commented on Sep 9, 2016

@TheFirstDeity

@langley-agm Just so you know, I don't believe this or #39 are related to type-bundling at all. According to the docs, .ts files take precedence over .d.ts files during module resolution at compile-time.

docs
...
...
docs
Source: https://www.typescriptlang.org/docs/handbook/module-resolution.html

In other words, core.ts takes precedence over core.d.ts at compile-time and it resolves the types by compiling the source.

langley-agm

langley-agm commented on Sep 9, 2016

@langley-agm
Contributor

Hi @TheFirstDeity

Just so you know, I don't believe this or #39 are related to type-bundling at all.

No one said otherwise =)

According to the docs, .ts files take precedence over .d.ts files during module resolution at compile-time.

This is why there's no core.ts file in the distribution now.

import { Logger } from 'angular2-logger/dist/es6/core';

This is not what we're aiming for.

If you have a look through node_modules@angular\core (or any of them) you'll notice there are no .ts source files anymore, they were removed. Maybe that's just how Angular libraries are supposed to be structured? Or maybe there's a setting to configure it...

I noticed. However no one knows how Angular libraries are supposed to be structured yet, like you just mentioned even the Angular team keeps changing this structure. As reference you can see other typescript libraries like RxJS do include the .ts files, It is useful to have those in the distribution.

30 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @vkniazeu@TheFirstDeity@ascreamingweas@langley-agm@JamesASmith83

        Issue actions

          ERROR in [default] \node_modules\angular2-logger\app\core\logger.ts:62:38 Type 'string' is not assignable to type 'Level'. · Issue #65 · code-chunks/angular2-logger