Skip to content

Difficulty with webpack multi-compiler and plugin hooks #273

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

Closed
swashata opened this issue Apr 29, 2019 · 6 comments
Closed

Difficulty with webpack multi-compiler and plugin hooks #273

swashata opened this issue Apr 29, 2019 · 6 comments

Comments

@swashata
Copy link
Contributor

Feature motivation

Right now, the way the plugin's hook system and webpack multi-compiler works, it makes it quite difficult to actually get the plugin hooks. For example, let's say we have

const ForkTsCheckerPlugin = require('fork-ts-checker-webpack-plugin');
const compiler = webpack([
	// array of webpack configuration with fork-ts-checker plugin
]);
const pluginHooks = ForkTsCheckerPlugin.getCompilerHooks(compiler);

// This won't work, because ForkTsCheckerPlugin didn't attach itself to
// the multi-compiler, rather the individual compilers
pluginHooks.receive.tap((d, l) => {
	console.log('done');
});

In such multi-compiler setup, we can not hook into fork-ts-checker hooks from the multi-compiler instance.

Feature description

However, we can loop over compilers property to get the actual compiler where the plugin is attached.

compiler.compilers.forEach(singleCompiler => {
	const pluginHooks = ForkTsCheckerPlugin.getCompilerHooks(singleCompiler);

	// This will work
	pluginHooks.receive.tap((d, l) => {
		console.log('done');
	});
});

Feature implementation

So I was thinking maybe the ForkTsCheckerPlugin.getCompilerHooks could be a bit intuitive to support multi-compiler out of the box. Internal implementation could be like this

  1. If compiler is a single instance, the don't change anything.
  2. If compiler is a multi-compiler instance, then loop over all compilers instance and add the callback on the same hook for every instance.

Maybe this would be a bit out of scope, but I think and helper would be really nice here. Something like

export function getMultiCompilerHooks(compiler) {
	if (! 'compilers' in compiler) {
		throw new Error('Works only with multi-compiler instance');
	}
	const hooks = [];
	compiler.compilers.forEach(singleCompiler => {
		hooks.push(getCompilerHooks(singleCompiler);
	});
	return hooks;
}

In any case, a little documentation on usage of the hooks would be really nice. I was trying to implement async typechecking and custom error reporting for a multi-compiler friendly webpack setup on my tooling and I really needed to dig through the source code to findout how and why things work.

@swashata swashata changed the title Difficulty with webpack multi-compiler Difficulty with webpack multi-compiler and plugin hooks Apr 29, 2019
@johnnyreilly
Copy link
Member

I wasn't actually aware of the multi compiler thing at all, so a docs PR would definitely be appreciated! Changing the behaviour in the way you suggest seems potentially useful. It would be backwards compatible by the sounds of it.

If you're game, maybe we could get a docs PR first. Merging that should be no problem at all.

Then if you wanted to raise a prospective pr which implements the changes you suggest and the team can collaborate with you on this? We'll see where it leads!

@swashata
Copy link
Contributor Author

That sounds like an idea. I will work on the docs first from my understanding. I will also show you the code which is responsible to properly hook into this plugin for both single compiler and multi-compiler. Maybe we can proceed from there.

@swashata
Copy link
Contributor Author

So this is the piece which determines whether it is a multi-compiler or not.

https://github.com/swashata/wp-webpack-script/blob/b355d9d8bc0815a7f408dd72bf9187299d49ef82/packages/scripts/src/scripts/Server.ts#L382-L393

			if ('compilers' in compiler) {
				// It is a multi-compiler instance
				// so tap the hook for every instance
				((compiler as any).compilers as webpack.Compiler[]).forEach(
					sCompiler => {
						this.addTsHooks(sCompiler, ForkTsCheckerWebpackPlugin);
					}
				);
			} else {
				// single compiler instance, so just tap one
				this.addTsHooks(compiler, ForkTsCheckerWebpackPlugin);
			}

I don't know of any other method to figure out the type of compiler it is. I will work on the docs for hooks.

@johnnyreilly
Copy link
Member

cc @sokra @TheLarkInn - do have any thoughts on this? Feels like there might be something worth considering in here around the general webpack compiler hooks API?

@piotr-oles piotr-oles mentioned this issue Apr 18, 2020
26 tasks
@piotr-oles
Copy link
Collaborator

@swashata
Please try fork-ts-checker-webpack-plugin@alpha - I've published a new version which automatically forwards all taps from the child compilers to the multi-compiler so we can use ForkTsCheckerWebpackPlugin.getCompilerHooks() with multi-compiler (see https://github.com/TypeStrong/fork-ts-checker-webpack-plugin/blob/v5.0.0-alpha.1/src/hooks/pluginHooks.ts#L46) 🚀
I will close this issue to clean-up the backlog. If this release didn't solve the issue, we can re-open this :)

@swashata
Copy link
Contributor Author

swashata commented Jun 4, 2020

Hi @piotr-oles I will take a look once I get a chance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants