Fix options caching when ts-loader is used in multiple rules

[yyx990803]

This is a bug we discovered in latest Vue CLI 3 beta (ref: vuejs/vue-cli#1399).

When the webpack config contains multiple rules that use ts-loader but with different options, the current option caching implementation will incorrectly always use the cached options from the first matched rule. This is because the caching is done by using only the webpack instance + the instance option as the key - it's not taking multiple rules into account.

This PR fixes it by ensuring each webpack instance + ts instance combo gets its own cache which caches validated options using a WeakMap.

The added test would fail without this fix as the options in the second rule will be ignored and appendTsxSuffixTo will take no effect.

[johnnyreilly]

Thanks @yyx990803 - I really appreciate the effort! I'm away right now. I'll take a look at this upon my return - hopefully this weekend.

[johnnyreilly]

Looks great - thanks! This will go out with the next release.

[johnnyreilly]

Shipped with: https://github.com/TypeStrong/ts-loader/releases/tag/v4.3.1

[johnnyreilly]

It looks like this change may have cause problems for vue users of the fork-ts-checker-webpack-plugin. See details here: vuejs/vue-cli#1399 (comment)

See also:

TypeStrong/fork-ts-checker-webpack-plugin#121

@yyx990803 would you be able to advise please? See my details against the vue-cli issue

[yyx990803]

@johnnyreilly

Found out the problem. Interestingly, this is because in vue-loader 14, <script lang="ts"> without any additional configuration loads the script block using ts-loader without any options. Now, due to the caching bug in ts-loader, it accidentally uses the cached options when it shouldn't, and turns out that makes everything work when technically it shouldn't. And the change I added in ts-loader 4.3.1 exposes this problem.

Note vue-loader v15 and above does not have this issue.

Unfortunately I can't think of a way to make this change completely backwards compatible for this scenario. We have a few options:

1. Keep the 4.3.1 change, but require vue-loader 14 users to change their config (they need to either explicitly pass the same ts-loader options via v14's loaders option, or upgrade to vue-loader 15).

2. Deprecate 4.3.1 and publish it as 5.0.0.

3. (not recommended) Revert the 4.3.1 change, and introduce an extra option (in addition to instance) for proper options caching. Note this would keep the previous behavior which is technically wrong.

[johnnyreilly]

Hey @yyx990803

Thanks for the thorough investigation! I agree that option 3 is a bad idea.

Given that the problem ts-loader 4.3.1 introduces only affects vue-loader 14, I'm inclined to do this:

• keep 4.3.1 as is
• change fork-ts-checker-webpack-plugin to run tests using vue-loader 15
• put information in the change log / release notes for 4.3.1 advising Vue users to upgrade to 15 or amend their config.

Does that sound good to you?

[yyx990803]

@johnnyreilly yeah, that sounds good to me.

[johnnyreilly]

So this is strange; I bumped the fork-ts-checker-webpack-plugin tests to use vue-loader 15 but the issue still seems to be present. You can see here:

https://travis-ci.org/Realytics/fork-ts-checker-webpack-plugin/jobs/387922073

Have I misunderstood something?

[yyx990803]

vue-loader 15 has breaking changes in terms of usage. I think it should be fixable by simply adding the required plugin (see here)

[johnnyreilly]

Great; all good now! In summary I've:

• Updated the 4.3.1 release of ts-loader and the changelog to advise the correct vue-loader usage with 4.3.1.

• Updated the tests for fork-ts-checker-webpack-plugin - I'd already merged my own PR before I noticed the one you'd submitted. I think it's good to have the test pack reflecting the usage of vue-loader 15 - and so I've closed your PR. My apologies for your wasted effort there; I hope that makes sense.

If there's anything else you think worth doing then do feel free to suggest it.

Thanks again for all your efforts @yyx990803 - you're a champ!

[yyx990803]

@johnnyreilly cool! No worries about the other PR, thanks!

[serloi]

Thanks for this post, you saved my night...
I updated ts-loader to 4.3.1 without updating vue-loader to v15, 2 hours I didn't understand why webpack transpilation not working... Rolled back to 4.3.0 and that's ok.
Note: I tried to migrate vue-loader to v15 and no success (webpack+vue+ts)...

[johnnyreilly]

If you want to work with Vue loader 14 and ts-loader 4.3.1 you could use the approach @yyx990803 illustrates here:

https://github.com/Realytics/fork-ts-checker-webpack-plugin/pull/122/files

[serloi]

Thank you for the link! I like to be up to date for all modules I use and I prefer to update to v15 in the same time as ts-loader 4.3.1.
Unfortunatly, v15 has breaking changes and to be honest I'm not very good in the set of webpack.config, so... I will report the modification of this file to update to v15 :(
have a good day!

[johnnyreilly]

In case it helps this link helped me update to vue-loader 15: https://vue-loader.vuejs.org/migrating.html#template-preprocessing

See how I applied these here: https://github.com/Realytics/fork-ts-checker-webpack-plugin/pull/123/files

[serloi]

Before try to switch to v15 and 4.4.0 (not 4.3.1 since today!), I tried to use your Fork module.
What a speed to transpile! after set parameters for vue (your doc is very good too!), all work fine, it's very cool :)
Thank you for your dev., really.

I know the link to migrate v15, I will retry to set my webpack config...

[yringler]

I just want to point out that requiring v15 is a breaking change.
My builds started breaking, and I spent an hour or two trying to figure out why, before narrowing in on ts-loader and seeing the release notes.
Now technically, looking at the symver spec, major version only needs to be updated if the API has a breaking change, but I still think that, if such an issue occurs again, you should consider bumping the major version.

[johnnyreilly]

I'm sorry that this caused you problems.

Rationale: the problem ts-loader 4.3.1 introduced only affected an older version of vue-loader. I figured that didn't quite merit the version bump.

Again, my apologies for your wasted time.