Fixed missing errors in watch mode in webpack5

[appzuka]

The fix in PR1200 was made to prevent deprecation warnings with webpack5, because it is no longer allowed to add assets during the afterCompile hook. These functions were moved to the new afterProcessAssets hook in webpack5. It seems that in watch mode, the afterProcessAssets hook is not called, so if any errors are introduced they will not be reported to webpack.

This PR allows the makeAfterCompile function to select whether assets should be added and/or errors should be reported. Running under webpack4 the behaviour is unchanged. Under webpack5 assets are added during afterProcessAssets and errors are reported during afterCompile. This should correct the behaviour observed in #1204.

This PR passes all comparison and execution tests, but as these run under webpack4 the webpack5 behaviour is not tested. I assume this will be corrected in the future when webpack5 is used to build ts-loader.

[johnnyreilly]

Do you think we should do caching here for perf reasons? The webpack version value won't change so regex-ing each time is perhaps less performant than if we cached it?

[appzuka]

This is only run once per compile cycle so I'm not sure how much difference it makes. I understand javascript regexes are not fast but I'm not sure if the impact is significant here. We could eliminate the regex by using

if (!loader._compilation.hooks.afterProcessAssets)

instead, as this does not exist in webpack4, but I thought checking webpack.version makes the code easier to understand. I assume that in the medium term there will be a webpack5 version of ts-loader so this is probably not a long term issue.

If you think caching is worthwhile I'll update the PR.

[johnnyreilly]

as an alternative to testing each time what do you think about doing something like this? (determining the function created based upon the webpack version) - this way we determine it once and we get the exact function we need each time:

// Adding assets in afterCompile is deprecated in webpack 5 so we
// need different behavior for webpack4 and 5
const addAssetHooks = !!webpack.version!.match(/^4.*/) ? (
  loader: webpack.loader.LoaderContext,
  instance: TSInstance
) => {
    // add makeAfterCompile with addAssets = true to emit assets and report errors
    loader._compiler.hooks.afterCompile.tapAsync(
      'ts-loader',
      makeAfterCompile(instance, true, true, instance.configFilePath)
    );
}
: (
  loader: webpack.loader.LoaderContext,
  instance: TSInstance
) => {
  // We must be running under webpack 5+

  // Add makeAfterCompile with addAssets = false to suppress emitting assets
  // during the afterCompile stage. Errors will be still be reported to webpack
  loader._compiler.hooks.afterCompile.tapAsync(
    'ts-loader',
    makeAfterCompile(instance, false, true, instance.configFilePath)
  );

  // Emit the assets at the afterProcessAssets stage
  loader._compilation.hooks.afterProcessAssets.tap('ts-loader', (_: any) => {
    makeAfterCompile(
      instance,
      true,
      false,
      instance.configFilePath
    )(loader._compilation, () => {
      return null;
    });
  });

  // It may be better to add assets at the processAssets stage (https://webpack.js.org/api/compilation-hooks/#processassets)
  // This requires Compilation.PROCESS_ASSETS_STAGE_ADDITIONAL, which does not exist in webpack4
  // Consider changing this when ts-loader is built using webpack5
};

[appzuka]

That is a neat solution. Are you going to make the change or do you want me to update the PR?

[johnnyreilly]

I'm not on my computer right now - so if you're able to do it that'd be amazing! But yeah - I'd be happy to do at some time over the weekend if you're busy. Happy either way

[johnnyreilly]

This is a tremendous PR! Great documentation in the code - it's really helpful. I've one question but otherwise I think this is marvellous!

[johnnyreilly]

Thanks @appzuka! This should ship with https://github.com/TypeStrong/ts-loader/releases/tag/v8.0.10