-
Notifications
You must be signed in to change notification settings - Fork 41
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 'string' is not assignable to type 'Level' #39
Comments
Hello @vcorr , Could you share your Typescript compiler version and your tsconfig options? |
absolutely,
"compilerOptions": {
}, |
@vcorr I was able to reproduce your issue when I use I ran some tests and this issue is not happening with the latest official release: However, I will leave this issue open in case it continues to occur after they officially release the next version, thank you ! |
I run into the same issue with Is it an error of the TypeScript compiler? |
@k7sleeper To clarify: It's an error thrown by the Typescript compiler. Not precisely an error of the compiler. The error is in the logger's code if the following official version of Typescript ships the way it is now ( in 1.9.0-dev ). If it does, it will get fixed appropriately. |
OK, I understand.Tx. I wonder why the compiler reads |
If I add a |
@k7sleeper |
That's clear! Setting the |
@k7sleeper sorry, miss-clicked the close button.
The typescript compiler doesn't use Also Just clarifying some things to avoid miss-conceptions. I'll be carefully reviewing your pull requests with more calm as soon as I get a chance. Thanks for contributing ! |
No, it also uses them for getting type information when processing
No! The The |
RxJS does include .ts files and so did Angular until apparently some releases ago. What other libraries are you basing on to make this statement ?
But they do exist, so why do you want to add this? |
This issue is caused because the Typescript compiler prefers the So, if you dont deliver the |
Generally, I think, delivering the If the non-executable code is delivered, IMO it should be located at a place in the package where the Typescript compiler does not look at by default. |
Actually this issue is caused by a change in Typescript syntaxis in a release that is not yet final. Removing or moving .ts is a workaround, not a fix, the underlying issue will still be there.
Since I've started using Angular 2 I've had to dive into the source code (.ts files ) so many times, I really see the value in delivering these files, I don't want to go to GitHub and look for the file manually every time I want to know how something works, its easier to just have the IDE take me to the file automatically because I already have it in my environment.
The fact that typescript is looking at .ts files of ignored libraries that it shouldn't compile when the .d.ts files exist it's an issue in Typescript side IMHO, I suggest you to open a ticket in their repo and add a link to this issue.
This is still a workaround but I'm not totally against this idea, I'll consider it when Typescript actually releases the version in which this issue is happening, who knows, maybe they'll fix it in some other way by then. |
getting this too on the beta release of Typescript 2.0 that was released last night. |
Hi @philjones88 Thanks for the uptate ! |
Is this ever going to be fixed? Are you accepting pull requests for that 1 line change? |
@philjones88 As I mentioned earlier, this issue is not happening in the latest official release version of Typescript (1.8), If you don't want to see this error I recommend using a version already released ( not alpha, nor beta ). I do accept pull requests, however I think the root cause of the issue is not this one line of code but the fact that typescript even tries to compile it when you already have it as a compiled dist. What if you wanted to keep your project using an old version of Typescript? I think @k7sleeper was on the right track trying to find a way for his project to avoid compiling this TP Library, however a way to do this without removing the source files from the package hasn't been found yet. If you find something like this and you make a pull request I'd be more than happy to take a look at it. |
Using 2.0 this is still broken, I don't think saying "don't use an alpha release" is really a solution |
@paralin I never said it was a solution my friend. I said 2.0 is not an officially released version and if you used one that was official you won't encounter this issue. By using alpha and beta releases you are accepting the fact that you will encounter this type of unsolved issues. The root cause of this issue is the fact that a project tries to compile a third party library when it shouldn't and personally I haven't found the solution for this if you are interested to pitch in with PRs or pointers of good information you are more than welcome to do so =) |
I solved it by just putting a + in front of that one statement as someone else said before. I have no problem with my app compiling the module. Maybe just release that as a stopgap fix so I don't have to use a git reference in my packages list. |
In my must humble opinion that's a workaround and doesn't fix the root cause, the fact that you don't have a problem with it doesn't mean it's correct. If by the time 2.0 is officially released there's no other solution, I'll apply that fix, until then I don't want to hide the underlying issue. I apologize for the inconveniences this causes. But like I mentioned, by using unnoficial releases you are accepting to deal with this kind of issues, which is only an error in the console that doesn't affect the lib's functionality at all. |
Regardless I don't see the problem with fixing your code regardless of if it's being interpreted at the time you compile the lib or I do. They are two separate issues. Just fix the one minor problem and leave the other issue to be fixed. |
The code is written using Typescript 1.8, and it works fine, there's nothing to fix. When you create a library using Java 7 you don't have to recompile it in order to use it with Java 8 projects do you? You don't suddenly need to convert all your classes into using lambdas instead just because a new version was released that supports them do you? The actual compiled lib code is JS and you can use it just fine. The source is there for reference not so you compile it. This is an issue in the Typescript side, not in this lib. You can raise a defect in the Typescript project if you wish to do so. |
I think you'll find that the moment more people than just yourself use a library, supporting other versions than the one you use will become something you'll want to do. Case and point. I'm not going to revert the entire Typescript version of my project just because you refuse to add a single character change to your repo.
This is NOT a Typescript bug. You're implicitly casting a string to a integer here. This is invalid, and if it wasn't warned about in a previous version of Typescript that's the bug. The fix is to cast the string value to an integer with a single character - the The sensible choice here is to just make the single character change. That's all you have to do to make this work with the newer versions of Typescript. It's really that simple. Just do it and save us all the trouble of using third party forks... |
You are completely missing the point. What I'm trying to say is that you can use a library build with Java 7 in a Java 8 project because the language supports it ! Your library doesn't have to support it, the library gets distributed as a jar file which you no longer have to compile cause its already compiled. Same with .ddls there's no reason to recompile a library that's already distributed as a compiled artifact. The fact that typescript makes you compile third party libraries IS a Typescript issue.
This is not invalid in Javascript nor in Typescript 1.8, its actually one of the biggest features/advantages of it. Its invalid in an unreleased version of Typescript with a new feature which I don't currently support because its not even released yet. The fact that Typescript doesn't support old versions of his own code its a lack in their side. |
I think you're missing the point. I'm saying if you make 1 commit that adds a single character |
Or... if you make a defect ticket in Typescript you might get this and many other future issues dissapear. I do know what you are saying, doesn't mean I have to agree with you. I already stated my point, I already heard yours and I really apologize for the troubles this is causing but you shouldn't be using alpha/beta releases in production environments, and if you are using them in personal/small projects, a warning in the console which doesn't affect functionality shouldn't matter. That's my stance and I agree to disagree with you. IMHO Software Quality is getting more affected by the day by things like using unreleased software in production projects, redoing instead of fixing, patching instead of finding the root cause. And I see this everyday in the daily tools I use. The common usage of unreleased software in production is affecting both companies and software developers alike. I might be wrong but this is what I've noticed and it's what I think, I do not want to encourage this. |
Typescript 2.0 is already released, 2.1 is the unreleased version right now, a lot of stuff is now using 2.0 as it IS compatible (for the most part) and has a lot of improvements. You're maintaining an extremely minor piece of software here man. You don't have the position to influence the entire industry as a whole as you seem to believe. Quit wasting your time on this crap, fix your bug (literally 1 character change), or refuse to do so, but don't try to lecture everyone on best practice. I'm not going to argue with you any more here - it's clear you'd rather argue best practice than fix your buggy code. To each his own, I guess. Best of luck with the project. |
@paralin The latest official release is 1.8.10 not 2.0.
No, I do not believe that, I just try to do my minor part.
I already refused several times =). I am not lecturing anyone I am trying to explain my reasoning for it. If you don't agree with it that's totally fine.
Thank you ! |
Hi, I am using Atom IDE with atom-typescript package [offered by MS]. The installed Typescript is: A) If I build my project under Atom IDE with F6 hotkey (I included your package of course, v0.3.0), the error message is raising. Why??? Maybe your tsconfig.json is different than mine or Atom use different settings for compilation. |
Hi @aperger I've noticed this in IDEs like IntelliJ where the IDE ignores your configuration and uses its own compiler: Probably Atom has a configuration like this where you can select the typescript version you want to use. Also remember that npm can install a typescript version locally and a different one globally, make sure the IDE is using the one you want or install the same version locally and globally. Those are the two things I can think of about why this could be happening to you. Let me know if one of them fixes it for you.
That link say its ok implicit conversion between integer and enum I agree, however it never says its not ok between strings and enum, that part its your own words. You can try this in your browser's console:
You'll see 2 gets printed. Newer unreleased versions of Typescript don't allow a String to be assigned to an enum anymore. Some people may prefer this, some people might not but that's the way it was designed and I want to respect that. If a new version of this lib is written in a newer version of Typescript, I'll make the correction for that version. As of now this library does not support a beta of Typescript. |
Thanks for the detailed explanation.... Additional note: |
It doesn't support a beta of Typescript despite requiring only a 1 character change to support the beta.... |
Only to recall: this problem is not caused because of this "one character", it's caused because TypeScript compiler compiles the Therefore, other libraries separate the source files ( Overall, it's good style to separate source files from generated files, and as a side effect it would solve this problem. |
EXACTLY ! This is the approach I will try first ( separating the definition files from the source ) when 2.0 gets released. |
Looks like this is an already known issue is Typescript: Sadly the apparent fix is only half implemented and still needs: I have commited a temporary manual fix and added a new checkbox in the welcome README in order to track a proper fix later on once Typescript has a way to do it automatically. |
I get this error for at least the version 0.2.3. I can't yet update to 0.3.0.
Line: /angular2-logger/app/core/logger.ts:59:38
Related to this? https://basarat.gitbooks.io/typescript/content/docs/tips/stringEnums.html
changing the line 59 from:
private _loadLevel = (): Level => localStorage.getItem( this._storeAs );
to
private _loadLevel = (): Level => +localStorage.getItem( this._storeAs );
Seems to fix it (converts string to number)
The text was updated successfully, but these errors were encountered: