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

Enhancement: TS compiles only dependencies instead of entire project #1378

Closed
wants to merge 3 commits into from

Conversation

colinskow
Copy link
Contributor

This PR improves the efficiency of the TypeScript build and resolves several issues that result from TypeScript compiling unnecessary files.

The current behavior is that on every build TypeScript compiles every single .ts file in the project tree. This is inefficient and can lead to issues when different files in the project require different configurations or use conflicting Typings.

The solution is to tell TypeScript specifically which entry files to compile, and it will automatically compile all their needed dependencies too. With this configuration TypeScript avoids compiling unnecessary files. awesome-typescript-loader doesn't seem to support passing in a configuration object, so this PR introduces a Webpack plugin which writes a temporary tsconfig.json file containing the correct entry files.

The only drawback is that lazy loaded modules need to be declared in src/app/lazy-loaded.ts or they will not get compiled. Maybe @shlomiassaf can write a script that automatically detects and adds them.

Overall this PR makes builds faster, avoids some Typings conflict issues, and makes watch mode more efficient by watching only necessary files. It is also needed for my Electron branch to separately watch the main and renderer builds.

@colinskow
Copy link
Contributor Author

Looks like Awesome TypeScript Loader is going to make this much easier to implement, so might as well wait until this issue is resolved before merging.
s-panferov/awesome-typescript-loader#364

@shlomiassaf
Copy link
Contributor

shlomiassaf commented Feb 7, 2017

@colinskow This introduces a lot of pitfalls.

The main one is the need to specify all of your lazy modules in a special file.
It makes lazy loading difficult more than it already is... lazy loading should be part of the route definition flow without extra requirements...
This alone is a blocker for me...

Also, the final tsconfig is not known to the user, very blurry...

I think it will create more confusion, we better wait for s-panferov/awesome-typescript-loader#364

@colinskow
Copy link
Contributor Author

@shlomiassaf

There are two ways TypeScript can work with lazy loaded modules:

  1. Compile every single file in the project
  2. Compile entry files, plus declared lazy loaded modules

If you want to avoid compiling everything, then there is no way around declaring lazy loaded modules. Remember when we used to have to declare vendor libraries?

The way around this would be to write a script that parses the dependency tree and detects lazy loaded modules on its own. But that's a lot of work for very little productivity gain in my eyes.

@shlomiassaf
Copy link
Contributor

@colinskow I'm not sure you are right, if this is the case lazy routes would never have worked and this is exactly why ng-router-loader exists.

Although ng-router-loader kicks in after awesome-typescript-loader finished TS to JS conversion it still adds a reference to a file (which is TS).
Webpack will load this file and send it through TS loader chain.

So anyway, the files goes through TS compilation hence TS type checking.

If awesome-typescript-loader will typecheck everything in the path of the entryPoint we're good.

@shlomiassaf
Copy link
Contributor

For example, ng-router-loader will convert this:

 { path: 'detail', loadChildren: './+detail#DetailModule'},

into this:

function() { return import('/forks/angular2-webpack-starter/src/app/+detail/index')  .then( function(module) { return module['DetailModule']; } ); }

Now this is pure JS code at this point but webpack will parse it to see if there are any require statements or import() in this case.

Since there are it will load them, since this points to a TS file it will go through the TS chain... all good.

@bmayen bmayen mentioned this pull request Apr 6, 2017
@PatrickJS
Copy link
Owner

@shlomiassaf @colinskow any updates on this PR? I really want to merge it in

@colinskow
Copy link
Contributor Author

@shlomiassaf the lazy loaded files are compiled because TypeScript's default behavior is to compile EVERY SINGLE FILE in the project tree. If we tell ATL to only compile entry files and their dependencies, it will skip the lazy loaded files unless they are declared explicitly.

This PR adds a bit of complexity to the build process, but also makes it go much quicker if there are a lot of unused TS files in the tree. (For example e2e and unit tests.)

It is absolutely necessary in my Electron fork since I have different builds for the main process, renderer process, Karma tests and Spectron tests, and the types conflict.

In this project it would probably give a couple second boost to each build, but nothing huge. The biggest benefit is you don't have to worry about type conflicts.

Two options:

  1. Declare lazy loaded modules in a separate file (existing behavior)
  2. Write a loader which resolves lazy loaded modules prior to ATL running

Also still waiting on s-panferov/awesome-typescript-loader#364 which will remove the need to write temporary .tsconfig files.

@PatrickJS PatrickJS closed this Sep 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should compiled/ directory be excluded by at-loader
3 participants