search

slackhq / csp-html-webpack-plugin

A plugin which, when combined with HTMLWebpackPlugin, adds CSP tags to the HTML output.
MIT License
164 stars 40 forks source link

Documentation tweaks #77

Closed maudnals closed 3 years ago

maudnals commented 3 years ago

Description

Hi again :) As a newcomer to this (great!) plugin, a few things in the README.md caused minor friction for me. I've listed a few suggestions for tweaks in README.md below. Not all of these may make sense, so feel free to point out to the ones you think don't or do!

  1. Would you consider adding const CspHtmlWebpackPlugin = require('csp-html-webpack-plugin'); for example in [this section](https://github.com/slackhq/csp-html-webpack-plugin#user-content-basic-usage:~:text=new%20HtmlWebpackPlugin())?
  2. In the full configuration example, consider replacing unsafe-eval with something else? To encourage the use of safe policies. And because if other developers are lazy like me, they first may copy the example policy into their code before editing it (and—pushing the idea a bit here—it's not impossible to forget to remove unsafe-eval). unsafe-inline feels more OK, since AFAIK it's needed in Safari.
  3. In the full configuration example, consider removing processFn: defaultProcessFn or adding an // optional comment above it? Why: as mentioned, my first reflex was to paste the full config example CSP to save myself some typing time. And so I ran into defaultProcessFn is not defined—which was a super quick one to fix, but it did require me to go back and edit my config. Maybe the question here is if you expect this feature to be used by most / a large part of this plugin's users? On the plus side though: this error was actually a good way for me to discover the feature / this ability to change the default method of what happens to the CSP after it has been created...

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

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

AnujRNair commented 3 years ago

@maudnals thanks for the feedback! I opened https://github.com/slackhq/csp-html-webpack-plugin/pull/78 with some changes to the readme, and would love your thoughts if you have some time.

maudnals commented 3 years ago

Great, thank you for the changes! Added a comment on the PR.

AnujRNair commented 3 years ago

Merged!