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

Typescript compile has errors (using the tsc version in the starter kit) #15

Closed
jonaskello opened this issue Sep 15, 2016 · 16 comments
Closed

Comments

@jonaskello
Copy link

jonaskello commented Sep 15, 2016

Nice work on the starter kit!

I add a npm script to run the typscript compiler in package.json like this:

{
  "scripts": {
    "tsc": "tsc -p src",
  }
}

Then when I run npm run tsc I get a bunch of compile errors.
There are some things missing in tsconfig.json. I'll try to do a PR to show more what I mean.

@jonaskello
Copy link
Author

jonaskello commented Sep 15, 2016

When the PR is applied these errors are left and I am not sure how to fix them:

$ npm run tsc

15 import Router from "./lib/Router";
node_modules/@types/react-router/index.d.ts(15,20): error TS2307: Cannot find module './lib/Router'.
16 import Link from "./lib/Link";
node_modules/@types/react-router/index.d.ts(16,18): error TS2307: Cannot find module './lib/Link'.
21 import Route from "./lib/Route";
node_modules/@types/react-router/index.d.ts(21,19): error TS2307: Cannot find module './lib/Route'.
26 import hashHistory from "./lib/hashHistory";
node_modules/@types/react-router/index.d.ts(26,25): error TS2307: Cannot find module './lib/hashHistory'.
2 import Calendar from 'react-input-calendar';
src/containers/currency-converter-container/components/currency-valuation-header-calendar.tsx(2,22): error TS2307: Cannot find module 'react-input-calendar'.
46 const actions = Object.assign({}, currencyConverterActions);
src/containers/currency-converter-container/index.tsx(46,24): error TS2339: Property 'assign' does not exist on type 'ObjectConstructor'.

@piotrwitek
Copy link
Owner

Hello,
As mentioned this project is powered by jspm which uses systemjs-builder and plugin-typescript for in browser transpilation for dev and build process for prod workflow.

The config you are mangling with is only used for IDE tooling, the real config for prod transpilation which you want to run is here: https://github.com/piotrwitek/react-redux-typescript-starter-kit/blob/master/jspm.config.js#L129

You could extract this config and run as seperate tsconfig and run tsc process, but why would you like to do it?
In that case, it doesn't make sense to use this starter-kit as it just gives you additional abstraction layer if you understand from where I'm coming from.

@jonaskello
Copy link
Author

Well, tsconfig.json is not primarily for IDE tooling but for the typescript compiler. The reason you would want to run the compiler is to check if you have any compilation errors. One scenario would be using notepad to edit the files and running tsc --watch --noEmit in a terminal to continuously check if you make any typing mistakes.

Coming from a strongly typed compiled language background I really like the idea of running a compiler and getting an error list from it. That list is the "truth". Tools like plugin-typescript run a subset of the typescript compiler via it's API and catches most errors but it is not the official compiler and does not catch all errors. For example, by default plugin-typescript does not do any type-checking. Yes, you can enable it in plugin-typescript but I don't want to because then the iteration time will be slower. So I can trade type-checking for faster iteration times. And if something seems funny, I run the offical compiler (tsc) to check for any hidden type errors or other errors that are possibly hidden by a bug in plugin-typescript. This way I get the best from both worlds.

I do agree that building for production scenario should use jspm, but still I would like to be able to run the official compiler to check for errors too. Can we have both?

@piotrwitek
Copy link
Owner

piotrwitek commented Sep 15, 2016

The config you are mangling with is only used for IDE tooling

Thanks for your explanation and I'm sorry, probably you didn't understand me, to be more explicit I was refering to the role of tsconfig in this project context, not in general :)

Basically my workflow for the scenario that you have explained is using Atom Editor with typescript-atom plugin, whenever I need to do your check 'truth' scenario I can try to run built-in compiler service with mentioned tsconfig file using F6 key, but to be honest I didn't dive that deep into plugin to check if it runs compiler service or tsc compiler process and if it really makes any difference between them.
From my experience it does it's job correctly, catching all the typing errors and emit fully working JS output files so I don't know what kind of hidden type errors you're referring to.

So with this setup you can have both scenarios, F6 for a full build based on tsconfig, and incremental builds with IntelliSense and all other features provided by TypeScript Language Service in Atom Editor by typescript-atom plugin.
The caveat is it's glued to typescript-atom plugin compiler version, which for me is not a problem.

I know it's not perfect solution and I am planning to sort it out in the future to become more flexible, but it's good enough right now and never did give me any trouble so I stay with that for the moment, but of course if you can figure out a better way your contribution will be really appreciated.

Regarding your compilation errors I'm just guessing that it could be related to module imports that are resolved by systemjs configs, I can see it's trying to resolve imports in ambient declarations but cannot find the real modules, I would check in that direction first.

@piotrwitek
Copy link
Owner

@jonaskello You made me curious, could you please elaborate what you mean by stating "plugin-typescript run a subset of the typescript compiler via it's API"?

Looking at the TypeScript Architecture Overview, tsc CLI (that you are referring to as TS Compiler) and language service that is used by plugin-typescript all in all are communicating with the same core TypeScript Compiler, and I don't understand in what sense it would run "a subset of a compiler", can you explain that?
Reference: https://github.com/Microsoft/TypeScript/wiki/Architectural-Overview

@jonaskello
Copy link
Author

On my team we use a mix of Webstorm, Sublime and vscode. They all have plug-ins for typescript but they can be a bit flaky. So even if the IDE is reporting one thing, running tsc may say something else. Running tsc also becomes the least common denominator for all members of the team, regardless of which IDE they use.

By hidden error I mean that if you clone this repo today and just run a simple tsc you will get 50+ compile errors. But I am guessing that you don't see these errors in your IDE. Thus the errors are "hidden". Even if you get errors from tsc, you may still be able to build and bundle the project using third party tools (jspm, webpack etc). But I would not feel comfortable deploying something that gives errors when compiling in tsc.

I think this repo is a really good starting point. I would star it twice if I could :-). I tried to set something like this up about a year ago but I had a hard time making it work so I went with webpack instead. I think people may clone this repo, run tsc, see a bunch of errors and draw the conclusion that it is broken (see for example #13). This would be a shame since the workflow this repo sets up is really nice. So what my PR is trying to do is to get a freshly cloned repo to compile without errors in tsc. The reason I setup tsc as an npm script is becuase it will then use the tsc installed locally in the project's node_modules, rather than the one installed globally (again see #13).

Maybe not related but I think the key to understanding the usefulness of the flow this repo sets up is that it is OK that the typescript files are not fully compiled and type checked in the browser. It would actually be OK to just strip the types (if you target es6). The reason that it is OK is that you have something running elsewhere doing the full checking, such as tsc --watch. This way you get really fast iterations with hot-realoading but can still have full checking in the background.

Regarding "subset of the typescript compiler" I did not mean the API does not expose the ability to do full project compile same as tsc. I am no expert on the API but I think plug-ins could do a full project compile like tsc if they wanted to but usually do a smaller check than tsc in the interest of speed. Also tools using the API can sometimes do more, for example typescript-loader for jspm could use the path mapping feature before tsc was using it. Another difference is that I think most IDE plug-ins only check single files, while tsc --watch always checks all files "for correctness".

@piotrwitek
Copy link
Owner

piotrwitek commented Sep 15, 2016

@jonaskello Thanks Jonas for sharing your experience, it's very interesting.
I totally agree with your approach for using tsc as a common denominator and I'm glad that you find this project useful 😄
I remember that In some earlier version it was working using tsc commands, but since it got more dependencies added, TS version updated and I started using @types instead of typings, and during all that time I was using only Atom so I lost count when it got broken.
Atom and it's typescript-plugin is really great because it is abstracting for me a lot of overhead related to incremental types checking (this is basically tsc --watch using TS language service) and as well running compilation with full type integrity check across project using tsconfig.
But I agree with you and I don't like that it makes this concept really opinionated and tightly coupled to Atom and it's compiler which is not good for usefulness and for scalability in bigger teams.
I'll probably take a look at it tomorrow and evaluate what I can do with it 💪
Thanks for motivation and valid points 😄

@piotrwitek
Copy link
Owner

Some findings:

@jonaskello
Copy link
Author

Yes I remember now I had trouble with tsc --watch and hot reloading earlier this year. I actually made a comment on the issue you mention. Basically you cannot do tsc --watch combined with hot reloading since it will write to all files and all will be hot reloaded.

So for now it would probably be best to just run tsc manually when you want to check that everything is OK. And for continuous checking use an IDE plug-in that only emits single files as you mention. Personally I use Webstorm and it can do some checking without re-emitting all files but it will not report project-wide errors (such as changing one file causes error in other files not open in the IDE) and it is a bit flaky sometimes and then I run tsc to see what is going on. Basically I think the workflow you propose is good, with the addition of being able to run tsc manually as needed.

@piotrwitek
Copy link
Owner

Ah ok so this is basically WebStorm TypeScript Language service limitation.
In Atom you can check for project-wide errors with build command as I mention, in my opinion it is working really solid, and if you create a new file or rename existing one you have a "Sync Typescript" command to fix that.

@jonaskello
Copy link
Author

I've also tried vscode which have the same limitation AFAIK. Webstorm has a few features that I find it hard to get by without, like optimize imports and auto-resolve import for es6 modules. I haven't tried atom lately, will give it a spin again.

Anyway, getting a bit derailed here :-). The point was that regardless of your choice of IDE you should also be able to do a simple tsc in the terminal without getting any compile errors.

@piotrwitek
Copy link
Owner

right, in my opinion should be doable

@piotrwitek
Copy link
Owner

tsc errors fixed and added new CLI based type-checking workflow

@piotrwitek
Copy link
Owner

NO-IDE workflow notes

@piotrwitek piotrwitek reopened this Sep 16, 2016
@piotrwitek
Copy link
Owner

piotrwitek commented Sep 16, 2016

@jonaskello reopening & waiting for your feedback if everything working fine :)

@jonaskello
Copy link
Author

Good notes in the readme. I did some testing working with sublime without any plug-in, just using tsc:watch for type checking. Worked perfectly, nice work! :-). I'll close as tsc dev-flow now works flawlessly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants