disableCspPlugin: true should not modify the template at all

phil-lgr commented 5 years ago

Description

Describe your issue here.

What type of issue is this? (place an `x` in one of the `[ ]`)

[x] bug
[ ] enhancement (feature request)
[ ] question
[ ] documentation related
[ ] testing related
[ ] discussion

Requirements (place an `x` in each of the `[ ]`)

[x ] I've read and understood the Contributing guidelines and have done my best effort to follow them.
[x ] I've read and agree to the Code of Conduct.
[x] I've searched for any related issues and avoided creating a duplicate issue.

Bug Report

Filling out the following details about bugs will help us solve your issue sooner.

Reproducible in:

slackhq/csp-html-webpack-plugin version: 2.5.0

node version: 10

OS version(s): Windows 10 & Mac OXS

Steps to reproduce:

set disableCspPlugin: true for files that are not index.html

const path = require('path');
const HtmlWebpackPlugin = require('html-webpack-plugin');

module.exports = new HtmlWebpackPlugin({
    template: './manifest.ejs',
    chunks: [],
    filename: 'manifest.json',
    inject: false,
    hash: false,
    minify: false,
    // csp plugin
    disableCspPlugin: true
});

// template ./manifest.ejs
<% const package = require('../../../package.json')%>
{
    "name": "<%=package.name%>",
}

run webpack compilation

Expected result:

csp-html-webpack-plugin does NOT modify my template since it is disabled

What you expected to happen:

{
    "name": "repo-name",
}

Actual result:

<html><head></head><body>{
    "name": "repo-name",
}
</body></html>

What actually happened

even though the user specify disableCspPlugin: true, the plugin still mutates the template:

phil-lgr commented 5 years ago

$.html() will inject <html><head></head><body></body></html>

TL;DR people use html-webpack-plugin for other uses, like creating a manifest.json, a browserconfig.xml or other so this plugin should not touch the template at all with using disableCspPlugin: true

phil-lgr commented 5 years ago

I understand you need to "clean" up some meta tag, but this should be:

opt-in
have a flag with a better name

the options would be:

disableCspPlugin: false // does not touch the template at all cleanMetaTagCspPlugin: false // if disableCspPlugin: true, also clean up the meta tags

phil-lgr commented 5 years ago

cc @AnujRNair @sibiraj-s if you guys are ok with the proposed changes, I can prepare a PR, it should probably be a major version bump since you seem to rely on disableCspPlugin: true to actually remove stuff from the templates

AnujRNair commented 5 years ago

@phil-lgr thanks for the feedback! I've been working on a new version of the CSP plugin, currently published on npm under an @beta tag - I made this change to exit early

Would you be able to npm i --save csp-html-webpack-plugin@3.0.0-beta.2 to see if this fixes your issue? Check the release notes for any other breaking changes too

Looking forward to hearing if this fixes the issue for you!

phil-lgr commented 5 years ago

Ok, I'll test tomorrow and report back, thanks for the quick reply

phil-lgr commented 5 years ago

I just tested and yes, this is great 👍 feel free to close now or when you merge the new changes

AnujRNair commented 5 years ago

Fixed in beta, which will soon become latest - to use this now, please install the beta version from npm

slackhq / csp-html-webpack-plugin