-
-
Notifications
You must be signed in to change notification settings - Fork 473
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
feat(addStyles): add support for __webpack_nonce__
#319
feat(addStyles): add support for __webpack_nonce__
#319
Conversation
Codecov Report
@@ Coverage Diff @@
## master #319 +/- ##
=======================================
Coverage 98.43% 98.43%
=======================================
Files 4 4
Lines 64 64
Branches 21 21
=======================================
Hits 63 63
Misses 1 1 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very good! Should we add nonce
by default, maybe better option for this?
Also i think we should add this into https://github.com/webpack-contrib/style-loader/blob/master/lib/addStyleUrl.js |
Adding it to |
@iamdavidfrancis yep, you are right /cc @webpack-contrib/org-maintainers should we add |
__webpack_nonce__
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As already mentioned, maybe better as an option (options.nonce
), but on the other hand doesn't seem to hurt to add it be default. Not sure about it...
@michael-ciniawsky let's leave this as is, |
Looking forward to this. It seems the previous workarounds of reusing nonced tags is no longer possible, so this is now the only legitimate fix for using style-loader with strict-dynamic CSP. |
Thanks for your work, I really need that! I use webpack to load my app inside an iframe. Does the Content Security Policy restrict access to global scope? Any ideas? function getNonce() {
if (typeof __webpack_nonce__ === 'undefined') {
return null;
}
var nonce = __webpack_nonce__;
if (!nonce && window.__webpack_nonce__) {
// This part should never be executed but in my case the message is logged 🤔
console.log('`__webpack_nonce__` did not return anything but `window.__webpack_nonce__` is set!')
nonce = window.__webpack_nonce__;
}
return nonce
} |
We spent more time on this issue and it works now without changing It is all about how to set This way it works: webpack.config.js: module.exports = {
entry: {
main: 'my/entry/index.js'
}
} my/entry/index.js __webpack_nonce__ = /* getting the nonce from somewhere */
// will compile to `__webpack_require__.nc = /* getting the nonce from somewhere */`
// you must not use import here as it is hoisted and executed first (https://github.com/webpack/webpack/issues/2776)
require('./theRealEntry.js') ./theRealEntry.js // `__webpack_nonce__` (compiled to `__webpack_require__.nc`is available in the style loader now
import('./styles.css')
// … |
...and it does not work at all if the |
@tobias-zucali thank you so much for your comment on HMR. That was the issue I was having. Im using webpack 4 and its still an issue. |
Somebody can create new issue and minimum reproducible test repo? |
@evilebottnawi I think its an issue with HMR not style-loader (or any other loader/project) that uses webpack_nonce. For example, I had the same issue with styled-components until I turned of HMR |
Will be great ensure this, need minimum reproducible test repo |
What kind of change does this PR introduce?
Feature to support
__webpack_nonce__
Did you add tests for your changes?
Yes, I added a test that verifies the nonce gets set on style tags. (Nonces will not help
link
tags as nonces are only used to allow inline styles.)If relevant, did you update the README?
I wasn't sure where in the README I should put a note about supporting
__webpack_nonce__
, so I have omitted it for now. I can add it in if requested.Summary
Currently if a site has a strict CSP that disallows
unsafe-inline
the rendered style tags from this loader will be ignored. Webpack supports adding a nonce via the__webpack_nonce__
property. This change looks for that property and adds the nonce attribute to generated style tags.More info here: #306
Does this PR introduce a breaking change?
This is only breaking if the package consumer was expecting the styles not to load, so it should not be an issue. If the consumer was already adding a nonce attribute via the options object, then that will take precedence over the
__webpack_nonce__
property, so it shouldn't break anyone doing it that way.