Allow to change module types from loaders - #10097

For maintainers only:

• This needs to be documented (issue in webpack/webpack.js.org will be filed when merged)
• This needs to be backported to webpack 4 (issue will be created when merged)

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

It looks like this Pull Request doesn't include enough test cases (based on Code Coverage analysis of the PR diff).

A PR need to be covered by tests if you add a new feature (we want to make sure that your feature is working) or if you fix a bug (we want to make sure that we don't run into a regression in future).

@Lyrkan Please check if this is appliable to your PR and if you can add more test cases.

Read the test readme for details how to write test cases.

"Windows node-12" check seems to be failing because of an unrelated timeout, and the code coverage issues are mainly due to already existing (and not covered) lines that were changed by Prettier.

I thought about that again, can came to the conclusion that it's probably a bad idea to have such a method in the loader.

This would probably lead to this call will be added to too many loader just in case the loader is used on some non-js file.

When multiple loaders are chained and set different module types this is getting a little bit weird, since only the last one is respected.

It increases the coupling from loader to webpack and webpack version.

Instead I think most problems can be solved by remaining the user to specify a type in the rules next the loaders.

Hi @sokra,

I agree with your concerns, but shouldn't there still be a way to force that kind of thing properly in some cases?

One use we would have for that method in Webpack Encore is that we created a custom loader which is only used in some require.context() calls.

Those are done internally so we do know that the loader will always be the last one in the chain but since currently there is no way (AFAIK) to set the type when using the inline syntax we had to resort to accessing/modifying this._module and this._compilation in it.

there is no way (AFAIK) to set the type when using the inline syntax

While you probably should avoid inline syntax anyway, there is a way.

You should use the inline match resource for this case. e.g. for a loader converting json to js:

import "./file.json.js!=!json-to-js-loader!./file.json"

This will interpret the result of the loader like it was in a file.json.js file, applying .js loaders and module type.

While you probably should avoid inline syntax anyway, there is a way.
You should use the inline match resource for this case. e.g. for a loader converting json to js:

Yeah, we're doing some weird things in order to copy files.. the copy-webpack-plugin couldn't work in our case so we rely on a temporary entrypoint with multiple require.context() calls matching whatever has to be copied (hence the inline syntax since we can't really add rules for them).

It's very hackish but surprisingly it works quite well (apart from people reporting that they couldn't copy JSON files).

I didn't think about the inline match resource though, I guess it should work, thanks for the tip :)

Edit: Actually I'm not sure it'll work in a require.resolve(...) require.context(), will try it out later :(

Actually I'm not sure it'll work in a require.resolve(...), will try it out later :(

should work fine.

should work fine.

I actually meant require.context() in my edit (long day...) for which it does not seem to work (but maybe I'm doing something wrong though).

Anyways, if this is possible in other "normal" use cases maybe you're right and this PR isn't needed after all, feel free to close it :)

Anyways, if this is possible in other "normal" use cases maybe you're right and this PR isn't needed after all, feel free to close it :)

Btw the quality of the PR is great. I just don't want to introduce a feature which affects the overall experience badly.