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

Webpack plugins don't get added #244

Closed
AndrewCraswell opened this issue Feb 2, 2021 · 11 comments
Closed

Webpack plugins don't get added #244

AndrewCraswell opened this issue Feb 2, 2021 · 11 comments

Comments

@AndrewCraswell
Copy link
Contributor

AndrewCraswell commented Feb 2, 2021

Craco: 6.1.0
CRA: 4.0.1

I've recently tried to add the CspHtmlWebpackPlugin to my craco.config.js file in order to dynamically inject a Content Security Policy into my site only during production builds. It seems the plugin is not being added correctly, and a number of other plugins I've tested don't get added either. This is especially odd because my config already had the BundleAnalyzerPlugin plugin being added which did work.

const { whenProd } = require("@craco/craco");
const CspHtmlWebpackPlugin = require("csp-html-webpack-plugin");
const HtmlWebpackPlugin = require("html-webpack-plugin");

module.exports = {
  webpack: {
    plugins: {
      add: [
        ...whenProd(() => [
          // None of these get added correctly
          new HtmlWebpackPlugin(),
          new CspHtmlWebpackPlugin(),
        ], []),
      ],
    },
  },
};

I've tried ejecting from CRA and adding the same plugins to the webpack config directly and everything works as I would expect. Maybe related to this recent change? #224

Here is a minimalistic repository that repros the issue:
https://github.com/AndrewCraswell/craco-bug

@alexasselin008
Copy link
Contributor

I'll look into it, and come back to you soon

@alexasselin008
Copy link
Contributor

@AndrewCraswell your repro case doesn't look like it have craco. Is it possible you forgot to push some of your code?

@AndrewCraswell
Copy link
Contributor Author

@alexasselin008 Sorry about that, I've just pushed an update

@b-zurg
Copy link

b-zurg commented Feb 3, 2021

I am having the same issue. Additionally remove is not working for me.

@alexasselin008
Copy link
Contributor

alexasselin008 commented Feb 3, 2021

@AndrewCraswell
Are you running npm run start or npm run build?

I looked at your sandbox and it should work fine if you use build.

I created a file outputWebpackConfig.js

const { createWebpackProdConfig } = require("@craco/craco");

process.env.NODE_ENV = "production"
const cracoConfig = require("./craco.config.js");
const webpackConfig = createWebpackProdConfig(cracoConfig, {},{verbose: true});

console.log(webpackConfig)

then i ran the following command line :

node outputWebpackConfig.js 

The script console logs the webpack config, where you can see the plugins and both plugin are added.

@b-zurg would you mind detailing your issue a bit more?

@AndrewCraswell
Copy link
Contributor Author

AndrewCraswell commented Feb 4, 2021

@alexasselin008 Thanks for the outputWebpackConfig.js script, that's a great utility for testing! The weird thing is the discrepancy between what Craco is reporting is in the config, and what's actually being executed. When you run yarn build you can verify that the CSP the plugin should inject into the index.html file is never added.

<meta http-equiv="Content-Security-Policy" content="base-uri 'self'; object-src 'none'; script-src 'unsafe-inline' 'self' 'unsafe-eval' 'nonce-5c504VCl7tqV5T87ckQxUA==' 'nonce-dU8Pu5WgPBXD7M02R2B+Gg==' 'nonce-SU0gjUzE62GbUHoh9Afdpw=='; style-src 'unsafe-inline' 'self' 'unsafe-eval' 'nonce-tL5Q1BDaTYqEUp4oCH/X1w=='">

However, if I eject, and add the same plugin in the same order that Craco reported having added it, everything works as expected and the plugin is executed.

I've added a PR to the repro repository to show how just ejecting and adding the plugin works:
https://github.com/AndrewCraswell/craco-bug/pull/1

It's worth mentioning that I've experienced this with a number of plugins as well, this isn't the only one.

@AndrewCraswell
Copy link
Contributor Author

AndrewCraswell commented Feb 4, 2021

As an aside, there's also some wonkiness with the Craco remove plugin functionality. Take for example this config:

module.exports = {
  webpack: {
    plugins: {
      remove: [...whenProd(() => ["HtmlWebpackPlugin"], [])],
      add: [
        ...whenProd(
          () => [
            new HtmlWebpackPlugin({
              inject: true,
              template: paths.appHtml,
              minify: {
                removeComments: true,
                collapseWhitespace: true,
                removeRedundantAttributes: true,
                useShortDoctype: true,
                removeEmptyAttributes: true,
                removeStyleLinkTypeAttributes: true,
                keepClosingSlash: true,
                minifyJS: true,
                minifyCSS: true,
                minifyURLs: true,
              },
            }),
            new CspHtmlWebpackPlugin(),
          ],
          []
        ),
      ],
    },
  },
};

My expectation is that this would remove the HtmlWebpackPlugin which comes default with CRA, then add the HtmlWebpackPlugin and CspHtmlWebpackPlugin plugins. What actually occurs is that the no instances of HtmlWebpackPlugin make it into the config at all.

@AndrewCraswell
Copy link
Contributor Author

AndrewCraswell commented Feb 6, 2021

It's hard to tell if this is a problem with Craco, or with HtmlWebpackPlugin. With more testing, I found that any plugin that requires HtmlWebpackPlugin doesn't work when added via the Craco config. But it does work if ejected and added to the Webpack config directly.

The only way I've been able to get these plugins to work from the Craco config is by removing the HtmlWebpackPlugin plugin from the Webpack config in CRA, and adding it into the Craco config. That is to say, it's not enough to just add a second copy of the plugin via Craco. Furthermore, when removing HtmlWebpackPlugin from the CRA config and adding via Craco, that will break the other plugins in the CRA Webpack config (InterpolateHtmlPlugin and InlineChunkHtmlPlugin both rely on it).

A workaround is to remove the CRA plugins via Craco, then also use Craco to add a new instance configured in an identical way, then add any new plugins (ex: CspHtmlWebpackPlugin) that also depend on HtmlWebpackPlugin. This is how it would look:

module.exports = {
  webpack: {
    plugins: {
      remove: [...whenProd(() => ['HtmlWebpackPlugin', 'InterpolateHtmlPlugin ', 'InlineChunkHtmlPlugin'], [])],
      add: [
        ...whenProd(
          () =>
            [
              new HtmlWebpackPlugin({
                inject: true,
                template: paths.appHtml,
                minify: {
                  removeComments: true,
                  collapseWhitespace: true,
                  removeRedundantAttributes: true,
                  useShortDoctype: true,
                  removeEmptyAttributes: true,
                  removeStyleLinkTypeAttributes: true,
                  keepClosingSlash: true,
                  minifyJS: true,
                  minifyCSS: true,
                  minifyURLs: true,
                },
              }),
              new InterpolateHtmlPlugin(HtmlWebpackPlugin, options),
              shouldInlineRuntimeChunk && new InlineChunkHtmlPlugin(HtmlWebpackPlugin, [/runtime-.+[.]js/]),
              new CspHtmlWebpackPlugin(),
            ].filter(Boolean),
          []
        ),
      ],
    },
  }
};

This gives the same functionality offered by CRA, but adds the CspHtmlWebpackPlugin plugin in a way where they all work.

This should work, except for the issue mentioned in the previous comment about the remove directive occurring after the add, making it impossible to replace any plugin defined in the CRA Webpack config. I've submitted a PR with a small change to resolve that and at least enable the workaround to function:
#245

@AndrewCraswell
Copy link
Contributor Author

AndrewCraswell commented Feb 6, 2021

Since the fix that enables a workaround has been merged, I'm going to close this. I suspect the issue is related to weirdness in the HtmlWebpackPlugin module. This theory is slightly confirmed seeing that Craco has its own plugin which wraps InterpolateHtmlPlugin and has to do special work to get it to function as expected.

@jordan-jarolim
Copy link

I think this issue might be related to this comment

So far I came accross 2 things:

  1. plugins added by craco.webpack.plugins.add are prepended to the beginning of plugins array. However, cspHtmlWebpackPlugin doc explicitly says it depends on hooks provided by htmlWebpackPlugin. So its necessary to first remove the plugin and then add it in the correct order.
  2. cspHtmlWebpackPlugin only generates hashes for scripts that are already in the html file (based on the comment above). Again, by only adding cspHtmlWebpackPlugin no hashes will be generated because InterpolateHtmlPlugin and InlineChunkHtmlPlugin are called later on, so there’s nothing to be signed.

What might solve the issue (feature request) would be pushing new plugins to the end of plugins array (or have this configurable.)

@jordan-jarolim
Copy link

So I would propose changing this

function addPlugins(webpackConfig, webpackPlugins) {    webpackConfig.plugins = webpackPlugins.concat(webpackConfig.plugins || []);}

into something like this:

function addPlugins(webpackConfig, webpackPlugins) {    webpackConfig.plugins = (webpackConfig.plugins || []).concat(webpackPlugins);}

I am not sure if this would be a breaking change tho.

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

No branches or pull requests

4 participants