Thoughts on "isolatedModules": true and ts-loader

[johnnyreilly]

Hey!

An idea just occurred to me which might improve performance still when using this plugin with ts-loader.

I've just been pondering usage of "isolatedModules": true, in relation to using ts-loader alongside fork-ts-checker-webpack-plugin. Essentially using this option will speed up compilation times. Which is nice.

However, if we set this option in the tsconfig.json then presumably fork-ts-checker-webpack-plugin will be impacted by this and effectively cease to typecheck. So that's not good.

Would you consider / would it make sense to have fork-ts-checker-webpack-plugin set isolatedModules to false regardless of what it says in the tsconfig.json? If that were the case then we could advise people using ts-loader with fork-ts-checker-webpack-plugin to use "isolatedModules": true, in their tsconfig.json for an extra perf boost.

What do you think?

cc @jamesbrantly

PS this is a badly documented compiler flag - there's a nice blog post on it though: http://weblogs.thinktecture.com/thomas/2016/05/tired-of-waiting-for-typescript-compilation.html

[piotr-oles]

Hi! :)
Thanks for this proposal, I think it's a good idea. Could you create PR for this? I think you will have to add some checks in lib/IncrementalChecker.js:29, it shouldn't be hard.

[johnnyreilly]

Sure - as soon as I get a moment I'll take a look!

[piotr-oles]

Released with 0.2.2 ;)

[johnnyreilly]

Sweet!

[odensc]

What does transpileOnly in ts-loader do different from isolatedModules? Or are they meant to work together?

[johnnyreilly]

@odensc - meant to work together. See the readme of ts-loader for transpileOnly details. Essentially it should just give a slight perf boost to ts-loader.

[odensc]

Ah, thanks.

I updated to 0.2.2 and enabled isolatedModules - however I'm getting errors such as:

ERROR at /home/oden/Dev/JS/fttv/node_modules/@types/babel-core/index.d.ts(16,19):
TS1205: Cannot re-export a type when the '--isolatedModules' flag is provided.

If isolatedModules is overriden to false, shouldn't I not be getting errors related to it being enabled?

[johnnyreilly]

No you shouldn't ... that's odd. Suggests that my change is not working? You definitely have 0.2.2? @piotr-oles is there anywhere else in the codebase where we need to make this change / is it being forgotten / overwritten?

[johnnyreilly]

This may be relevant: microsoft/TypeScript#2812 Giving it a brief read I'm wondering if there's a problem using isolatedModules in conjunction with @types. I don't know enough about the flag to say. May ask a question on the TypeScript repo

[odensc]

Yes - node_modules/fork-ts-checker-webpack-plugin/package.json is "version": "0.2.2".

It seems the change was made in microsoft/TypeScript#15538. I'm actually running 2.4.0-dev which has that change, so if you tested with 2.3 you wouldn't see the error. But it will start breaking when 2.4 is released.

Still, none of this should be an issue if isolatedModules is overriden to false.

[johnnyreilly]

Question: do you get the same error if instead of using transpileOnly you use happyPackMode? (This disables error reporting in ts-loader entirely) I'm trying to work out if the error is coming from ts-loader or fork-ts-checker-webpack-plugin.

[odensc]

@johnnyreilly Same error with happyPackMode. (it also isn't emitting a webpack error, so it must be the plugin)

[johnnyreilly]

OK - we've ruled out ts-loader at least which is good. Let's see if @piotr-oles can provide any clues when he comes online. I've raised an issue with the typescript repo to get guidance.

[johnnyreilly]

OK - the issue is actually in the TypeScript compiler. Also, it turns out that my understanding of isolatedModules was backwards and actually it's not useful to us when working with fork-ts-checker-webpack-plugin at all. 😞 I'll update the ts-loader docs to reflect that.

The change in my PR doesn't really add much of use; although it arguably does no no harm since there's no reason you'd ever want to run fork-ts-checker-webpack-plugin with isolatedModules set to true. If you'd like me to remove it I'm happy to submit another PR that reverts the change. Your call

Either way; my apologies for wasting your time on this one. Not my intention.

[odensc]

No problem, at least I know what it does now. :)

[piotr-oles]

@johnnyreilly Yes, I think it would be good to revert this changes :) One place less to introduce weird bugs in the future :)