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

Update definition files in watch mode in webpack@5 #1249

Merged
merged 3 commits into from
Feb 4, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ These options should be functions which will be used to resolve the import state
#### getCustomTransformers
| Type |
|------|
| ` (program: Program) => { before?: TransformerFactory<SourceFile>[]; after?: TransformerFactory<SourceFile>[]; } ` |
| ` (program: Program) => { before?: TransformerFactory<SourceFile>[]; after?: TransformerFactory<SourceFile>[]; afterDeclarations?: TransformerFactory<SourceFile>[]; } ` |

Provide custom transformers - only compatible with TypeScript 2.3+ (and 2.4 if using `transpileOnly` mode). For example usage take a look at [typescript-plugin-styled-components](https://github.com/Igorbek/typescript-plugin-styled-components) or our [test](test/comparison-tests/customTransformer).

Expand Down
38 changes: 25 additions & 13 deletions src/instances.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,21 +336,33 @@ const addAssetHooks = !!webpack.version!.match(/^4.*/)
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;
});
}
// makeAfterCompile is a closure. It returns a function which closes over the variable checkAllFilesForErrors
// We need to get the function once and then reuse it, otherwise it will be recreated each time
// and all files will always be checked.
const cachedMakeAfterCompile = makeAfterCompile(
instance,
true,
false,
instance.configFilePath
);

// compilation is actually of type webpack.compilation.Compilation, but afterProcessAssets
// only exists in webpack5 and at the time of writing ts-loader is built using webpack4
const makeAssetsCallback = (compilation: any) => {
compilation.hooks.afterProcessAssets.tap('ts-loader', () =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this code block is only accessible for Webpack 5 you can use the new hook:

compilation.hooks.processAssets.tap({ name: 'ts-loader', stage: webpack.Compilation.PROCESS_ASSETS_STAGE_ADDITIONAL }, (_assets) => {
...
}

Webpack is already required, so no other changes needed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ts-loader is currently built using webpack@4.5.0, which does not define webpack.Compilation.PROCESS_ASSETS_STAGE_ADDITIONAL, so although the code would run under webpack 4 and 5 it cannot be built under webpack4 if we include this.

Also, I think it is OK to use the afterProcessAssets hook. The various processAssets hooks provide a list of the assets that are being processed at that stage, but we are not using the list of assets. We just need a place to hook into where we can emit the assets and afterProcessAssets seems to work fine.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, please use compilation.hooks.processAssets, you can check webpack version using compiler.webpack (if exists it is webpack@5, if no it is webpack@4), also using compilation.hooks.processAssets I can implement cache, so it will speed build in watch mode

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried building ts-loader using compilation.hooks.processAssets but it will not build because webpack.Compilation.PROCESS_ASSETS_STAGE_ADDITIONAL is not defined in webpack 4. If you try to build using webpack 5 there are other errors. I assume at some point in the future ts-loader will upgrade to webpack 5 but until then we cannot use anything not defined in webpack 4. afterProcessAssets is a valid webpack 5 hook so I'm not sure there is any reason to avoid using it.

Copy link
Contributor

@alexander-akait alexander-akait Feb 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example of usage https://github.com/webpack-contrib/terser-webpack-plugin/blob/v4.2.3/src/index.js#L647 (webpack@4 and webpack@5), on this (afterProcessAssets) hook I can't help with cache, so if you want speedup your build and do developers happy - OK

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just note: maybe to do new major release with dropping webpack@4 is not bad idea

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah - I'm in favour

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(not in this PR obviously)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I can help with cache and optimizations ⭐

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks in advance!

cachedMakeAfterCompile(compilation, () => {
return null;
})
);
};

// We need to add the hook above for each run.
// For the first run, we just need to add the hook to loader._compilation
makeAssetsCallback(loader._compilation);

// For future calls in watch mode we need to watch for a new compilation and add the hook
loader._compiler.hooks.compilation.tap('ts-loader', makeAssetsCallback);

// 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
Expand Down