webpack-require-from icon indicating copy to clipboard operation
webpack-require-from copied to clipboard

Add feature to disable plugin for css files

Open ppiyush13 opened this issue 4 years ago • 8 comments

Fixes #9

Whenever webpack-require-from plugin is used with mini-css-extract-plugin, it logs errors in console as shown below:

image

To my surprise, webpack compilation completes successfully.

On further inspection I found that mini-css-extract-plugin does not honors public path overridded by webpack-require-from plugin. mini-css-extract-plugin has is own options to set public path for CSS files.

webpack-require-from will log warnings for each import or url CSS rules. So sometimes it becomes really painful to browse through the compilation logs. One way to solve this is to set suppressErrors to true. But this will suppress all errors in production as well for JS public path.

So I propose the option disableCss to avoid compilation of CSS files by mini-css-extract-plugin with extensions .css, .scss, .sass . For backward compatibility disableCss will be false by default.
Developers can set public paths for CSS files using configurations provided by mini-css-extract-plugin.

One can have exact same experience with extract-css-chunks-webpack-plugin as well.

I have created git repository to test and reproduce this issue, https://github.com/ppiyush13/webpack-require-from-example/blob/master/readme.md

ppiyush13 avatar Feb 08 '20 16:02 ppiyush13

@ppiyush13 thanks a lot for identifying and suggesting this enhancement! The PR looks good!

I would suggest to extend the feature to something like "exclude" filter - e.g. developer provides regex for the plugin to "skip" certain files.

Would you have time and will to enhance you PR with that proposed suggestion?

agoldis avatar Feb 09 '20 00:02 agoldis

@agoldis I first thought of adding something general purpose solution like "filter" option.

  • For CSS, child compilation name attribute is set to <plugin_name> <css_file>. Example: 'mini-css-extract-plugin c:\\index.css' 'extract-css-chunks-webpack-plugin /root//app.css'

    I have leveraged this behavior to my advantage to skip compilation for CSS files by testing compilation name to end with CSS file extensions.

  • For other file types, this information is not available and we can't identify which files are being compiled. For example, main webpack compiler does not set any name, 'html-webpack-plugin' sets name as 'html-webpack-plugin'. So name is not sufficient to identify files and skip compilation.

    I tried exploring for few more attributes available on compilation, but I could not figure some definite way achieving our purpose.

I am missing something here ? Can we achieve this by tapping into some other hooks ? With your guidance I can generalize this further by adding "exclude" option.

ppiyush13 avatar Feb 09 '20 07:02 ppiyush13

Yeah, you're right, I was sure for some reason that compilation target name is available for each compilation. I want to dig deeper into it, is your PR urgent? I wanted to write a test and give another chance for the filter option

agoldis avatar Feb 10 '20 05:02 agoldis

@agoldis I think we should work towards proper solution. Anyways we have workaround to avoid warnings with "suppressErrors".

Kindly let me know if you need any help.

ppiyush13 avatar Feb 10 '20 06:02 ppiyush13

Hey @ppiyush13! Finally had some time today. Seems like we can use mainTemplate hooks argument chunk, specifically chunk.entryModule.resource contains the full path to the resource that is associated with the chunk.

For example (from your https://github.com/ppiyush13/webpack-require-from-example):

  • for html-webpack-plugin it is /Users/agoldis/webpack-require-from-example/public/index.html
  • for mini-css-exract-plugin it is /Users/agoldis/webpack-require-from-example/micro-frontend/src/app.CSS

Seems like we can use this piece of data to bailing out and filter out files based on path, e.g. https://github.com/agoldis/webpack-require-from/blob/master/src/plugin.js#L134

Today the plugin uses several hooks, depending on configuration option being used (and webpack version), so the naive solution would be to filter in each such hook. Another options would be heavier refactor to allow more elegant filtering, but I don't think that is right timing.

Please tell me if you're up to handle the implementation.

Ideally, I'd love to have some tests in place. The might seems a bit tricky, so I can handle it.


About your PR - it seems you've invested time and effort creating the example and a solution - I appreciate it a lot and it was very helpful, really!

The implementation as is, is likely to create some confusion, it doesn't really "disables CSS" warnings. Depending on plugin, the filename might or might not be in compilation name, so people will start opening issues, expecting all the CSS files to be silenced.

That's why, I will probably have to decline the PR's in its current state :( Sorry about that, I really appreciate the work you've done!

agoldis avatar Feb 18 '20 05:02 agoldis

@agoldis

Good to know that my example repo was very helpful !
Thanks for finding some alternate way to find out exact resource path. Can you share some code snippet here on how to get chunk argument from mainTemplate hooks ?

I totally understand that current implementation might create issue if file name is not available in compilation name. So I am ready to change my implementation. But before that, I have few points for consideration:

  1. Why developer would need to filter files ?
    This plugin adds some code to override default __webpack__p (webpack publicPath) in runtime chunk. So this plugin operates over webpack compilation rather than on individual files. So what is the exact need for the filter option ?
    Can compilation still be skipped on getting chunk.entryModule.resource ? I think it wont be possible if there are no sync hook available to get chunk argument. I might be missing something, please correct me if I am wrong.

  2. For which asset types this plugin is applicable to ? This plugin is applicable where other resources are imported. IMO only JS, CSS and HTML assets are capable of importing other resources. In JS, other resources can be any other asset. In CSS, other resources could be font, images resources. In HTML. other resources could be any other asset.

    Please correct me if there are any more such assets.

  3. Dont you think, this plugin must operate only for parent compilation. For all other child compilations (like html-webpack-plugin, mini-css-exract-plugin), this plugin is not needed. Because only parent runtime will be executed in browser. We must provide option to skip child compilations. What do you think ?

What are your opinions ?

ppiyush13 avatar Feb 19 '20 16:02 ppiyush13

@ppiyush13 Hey! I am happy to have this conversation with you! Really good questions!

Now, due to the specifics of implementation, there's a problem. The plugins "infects" webpack compilation process and causes warnings. That's because it needs some global variables to exist, which aren't there when used internally by other plugins.

The ideal solution would be to eliminate those errors by "injecting" required code for each compilation, so the methods / variables would always be there.

I tried to implement that, but gave up... Also there weren't many complaints about that.

Why developer would need to filter files?

I don't think we need filtering per se. It's just one of the ways to deal with the issue you've discovered. Plugin's premise is to modify dynamically fetched path at runtime. Today dev can "skip" modifying paths for specific assets by using those:

  • https://github.com/agoldis/webpack-require-from#methodname
  • https://github.com/agoldis/webpack-require-from#replacesrcmethodname

For which asset types this plugin is applicable to?

Ideally, for all remote resources fetched at runtime

Dont you think, this plugin must operate only for parent compilation?

No, there're cases when child compilations should also be altered. E.g. https://medium.com/@prateekbh/my-experience-writing-a-webpacks-child-compiler-plugin-a1237c175947 describes how assets are duplication with a different configuration.

agoldis avatar Feb 21 '20 23:02 agoldis

@agoldis ,

That's because it needs some global variables to exist, which aren't there when used internally by other plugins.

You are right, ideal solution would be inject the required methods / variables to each compilation. But these required methods / variables are known at the run-time. So we may not be able to inject these to each compilation at compile time.

No, there're cases when child compilations should also be altered.

I didn't thought about this situation. You are correct, there might be cases where child compilation is used to duplicate JS assets with different configurations. Thanks for making me aware about this.

But what I really meant in last comment is webpack-require-from operate at compilation level rather than at file level. So we should provide some means to ignore compilation. For example in my case I don't want this plugin to operate (inject code to change webpack public path) for mini-css-exract-plugin (which has its own compilation). But I would want it operate for babel-esm-plugin (has its own compilation)

Whats your opinion ?

ppiyush13 avatar Mar 13 '20 17:03 ppiyush13