Add support for custom tag attribute #155

felixmosh · 2016-12-10T21:57:54Z

What kind of change does this PR introduce?
feature, allows to add additional attributes on the style / link element that the loader creates

Did you add tests for your changes?
No, tests
If relevant, did you update the README?
Update README
Summary

This PR allows to add id attribute or any custom attribute, in my case i have runtime css style processor that relies on custom attribute on the style tag that it should process.

Does this PR introduce a breaking change?
no breaking changes

Other information

Fixes #147

jsf-clabot · 2016-12-10T21:57:55Z

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 4 committers have signed the CLA.

✅ d3viant0ne
❌ felixmosh
❌ blainekasten
❌ TheLarkInn

felixmosh · 2016-12-11T19:05:23Z

@SpaceK33z can you review it?

felixmosh · 2016-12-15T08:00:48Z

someone is alive here?

sokra · 2016-12-23T14:12:41Z

addStyles.js

@@ -26,6 +26,8 @@ module.exports = function(list, options) {
 	}

 	options = options || {};
+  options.tagAttrs = typeof options.tagAttrs === "object"? options.tagAttrs : {};


felixmosh · 2016-12-23T14:19:13Z

Done, thanx

sokra · 2016-12-23T14:22:06Z

the spacing should be tabs instead of spaces

felixmosh · 2016-12-23T14:26:19Z

Fixed the spacing

SpaceK33z · 2016-12-23T14:26:58Z

README.md

@@ -61,6 +61,10 @@ By default, the style-loader appends `<style>` elements to the end of the `<head

 If defined, the style-loader will re-use a single `<style>` element, instead of adding/removing individual elements for each required module. **Note:** this option is on by default in IE9, which has strict limitations on the number of style tags allowed on a page. You can enable or disable it with the singleton query parameter (`?singleton` or `?-singleton`).

+#### `tagAttrs`
+
+If defined, style-loader will attach given attributes with there values on `<style>` / `<link>` element.


there -> their

Can you also add what this option expects? Is it an array or object? And what do I exactly need to define in that? A simple example would already help a lot.

felixmosh · 2016-12-24T11:53:22Z

All done...

gruberjl · 2017-01-09T03:44:20Z

This is exactly what I needed, thanks.

FYI: The docs say require('style-loader?{tagAttrs:{id: 'style-tag-id'}}!style.scss');. I think style-tag-id should be wrapped in double quotes. It did work what I used the options syntax in Webpack 2.

{
    loader: 'style-loader',
    options: {
        singleton: true,
        tagAttrs: {'amp-custom': ''}
    }
}

felixmosh · 2017-01-09T07:47:39Z

@gruberjl, you are right, fixed it.
Thanx.

This is exactly what i needed also, i`m waiting for the owner of the proj to approve it :\

felixmosh · 2017-01-19T08:08:37Z

Someone?

joshwiens · 2017-01-19T08:22:36Z

@felixmosh - Give me a minute, I'll see if I can find someone though everyone is still pretty busy with the 2.2.0 final release of Webpack.

joshwiens · 2017-01-19T08:28:22Z

addStyles.js

-	linkElement.rel = "stylesheet";
+	options.tagAttrs.rel = "stylesheet";
+
+	attachTagAttrs(styleElement, options.tagAttrs);
 	insertStyleElement(options, linkElement);
 	return linkElement;
 }



It might not hurt to throw a quick // document this function in there given it's an optional feature.

Sorry, i didn`t understood, what does it means "to throw a quick // document this function"

dsebastien · 2017-02-01T06:19:08Z

This will be very nice to have as it should allow specifying a nonce to be used with a content security policy (CSP).

michael-ciniawsky · 2017-03-06T15:49:15Z

README.md

@@ -61,7 +72,17 @@ By default, the style-loader appends `<style>` elements to the end of the `<head

 If defined, the style-loader will re-use a single `<style>` element, instead of adding/removing individual elements for each required module. **Note:** this option is on by default in IE9, which has strict limitations on the number of style tags allowed on a page. You can enable or disable it with the singleton query parameter (`?singleton` or `?-singleton`).

-## Recommended configuration
+#### `tagAttrs`


tagAttrs => Attrs/Attributes

michael-ciniawsky · 2017-03-06T15:50:19Z

addStyles.js

@@ -26,6 +26,8 @@ module.exports = function(list, options) {
 	}

 	options = options || {};
+	options.tagAttrs = typeof options.tagAttrs === "object" ? options.tagAttrs : {};


tagAttrs => attrs/attributes

michael-ciniawsky · 2017-03-06T15:53:47Z

addStyles.js

 	insertStyleElement(options, linkElement);
 	return linkElement;
 }

+function attachTagAttrs(element, attrs) {
+	for(var key in attrs) {


options.attrs.forEach((attr) => element.setAttribute(attr, attrs[attr]));

options.attrs is not an array but an key-value pair (object).
So forEach won't work.
I will change it to
Object.keys(options.attrs).forEach...

And because of the fact that this code gonna run on the browser, i prefer to omit the arrow func.

michael-ciniawsky · 2017-03-06T17:17:23Z

Related #119

joshwiens · 2017-03-06T20:53:07Z

So I hate to tell you this but it looks like there was an issue updating this PR around Jan 9th ( oops on a rebase maybe ).

Point being, the CLA assistant is currently showing 4 contributors to this PR which isn't going to work.

Simplest ( and probably only ) way to fix this is to copy code changes to a new branch, Open a new PR, reference this one & then close it. We will finish this and get it merged as soon as we have a pull request that doesn't have the CLA issues.

felixmosh · 2017-03-06T20:55:38Z

So should i create a PR from scratch?

joshwiens · 2017-03-06T21:01:42Z

To prevent issues, it just needs to be a new branch. Everything ( all the changes from this PR ) can be committed in one hash.

We will reference this PR from the new one, so none of the review comments will be lost.

madjam002 · 2018-03-06T11:22:21Z

@dsebastien The way that custom tag attributes have been implemented here is not suitable for implementing CSP nonce attributes. A nonce by definition is a secure random string that can only be used once [1], and by hard coding a nonce in your webpack config goes against this. By using the same nonce over and over again, malicious entities know what the nonce will be every time which defeats the point of CSP in the first place.

[1] https://en.wikipedia.org/wiki/Cryptographic_nonce

felixmosh · 2018-03-06T17:15:47Z

@madjam002, looks like the custom attribute feature is not suitable for that job, pls don't use it for that.

sokra reviewed Dec 23, 2016

View reviewed changes

felixmosh force-pushed the master branch from 3ac9caf to 8ddcb1b Compare December 23, 2016 14:17

felixmosh force-pushed the master branch from 8ddcb1b to d327daf Compare December 23, 2016 14:25

SpaceK33z reviewed Dec 23, 2016

View reviewed changes

felixmosh force-pushed the master branch from d327daf to b65c2ec Compare December 23, 2016 14:28

felixmosh force-pushed the master branch from 015e39b to dad6608 Compare January 9, 2017 07:46

joshwiens reviewed Jan 19, 2017

View reviewed changes

dsebastien mentioned this pull request Feb 1, 2017

Issue with a strict Content Security Policy (CSP) ui-router/visualizer#12

Closed

michael-ciniawsky added this to Feature in Dashboard Mar 6, 2017

michael-ciniawsky requested changes Mar 6, 2017

View reviewed changes

michael-ciniawsky mentioned this pull request Mar 6, 2017

load css with media=all attribute #147

Closed

felixmosh closed this Mar 6, 2017

felixmosh force-pushed the master branch from 007f479 to 029db64 Compare March 6, 2017 21:39

felixmosh mentioned this pull request Mar 6, 2017

feat(tag-attribute): Add support for custom tag attribute #183

Merged

michael-ciniawsky removed this from Feature in Dashboard Mar 6, 2017

madjam002 mentioned this pull request Mar 6, 2018

__webpack_nonce__ support or alternative way of solving CSP? #306

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for custom tag attribute #155

Add support for custom tag attribute #155

felixmosh commented Dec 10, 2016 •

edited by SpaceK33z

jsf-clabot commented Dec 10, 2016 •

edited

felixmosh commented Dec 11, 2016 •

edited

felixmosh commented Dec 15, 2016

sokra Dec 23, 2016

felixmosh commented Dec 23, 2016

sokra commented Dec 23, 2016

felixmosh commented Dec 23, 2016

SpaceK33z Dec 23, 2016

SpaceK33z Dec 23, 2016 •

edited

felixmosh commented Dec 24, 2016

gruberjl commented Jan 9, 2017

felixmosh commented Jan 9, 2017 •

edited

felixmosh commented Jan 19, 2017

joshwiens commented Jan 19, 2017 •

edited

joshwiens Jan 19, 2017

felixmosh Jan 19, 2017

dsebastien commented Feb 1, 2017

michael-ciniawsky Mar 6, 2017

michael-ciniawsky Mar 6, 2017

michael-ciniawsky Mar 6, 2017

felixmosh Mar 6, 2017 •

edited

michael-ciniawsky commented Mar 6, 2017

joshwiens commented Mar 6, 2017

felixmosh commented Mar 6, 2017

joshwiens commented Mar 6, 2017

madjam002 commented Mar 6, 2018

felixmosh commented Mar 6, 2018

Add support for custom tag attribute #155

Add support for custom tag attribute #155

Conversation

felixmosh commented Dec 10, 2016 • edited by SpaceK33z

jsf-clabot commented Dec 10, 2016 • edited

felixmosh commented Dec 11, 2016 • edited

felixmosh commented Dec 15, 2016

sokra Dec 23, 2016

Choose a reason for hiding this comment

felixmosh commented Dec 23, 2016

sokra commented Dec 23, 2016

felixmosh commented Dec 23, 2016

SpaceK33z Dec 23, 2016

Choose a reason for hiding this comment

SpaceK33z Dec 23, 2016 • edited

Choose a reason for hiding this comment

felixmosh commented Dec 24, 2016

gruberjl commented Jan 9, 2017

felixmosh commented Jan 9, 2017 • edited

felixmosh commented Jan 19, 2017

joshwiens commented Jan 19, 2017 • edited

joshwiens Jan 19, 2017

Choose a reason for hiding this comment

felixmosh Jan 19, 2017

Choose a reason for hiding this comment

dsebastien commented Feb 1, 2017

michael-ciniawsky Mar 6, 2017

Choose a reason for hiding this comment

michael-ciniawsky Mar 6, 2017

Choose a reason for hiding this comment

michael-ciniawsky Mar 6, 2017

Choose a reason for hiding this comment

felixmosh Mar 6, 2017 • edited

Choose a reason for hiding this comment

michael-ciniawsky commented Mar 6, 2017

joshwiens commented Mar 6, 2017

felixmosh commented Mar 6, 2017

joshwiens commented Mar 6, 2017

madjam002 commented Mar 6, 2018

felixmosh commented Mar 6, 2018

felixmosh commented Dec 10, 2016 •

edited by SpaceK33z

jsf-clabot commented Dec 10, 2016 •

edited

felixmosh commented Dec 11, 2016 •

edited

SpaceK33z Dec 23, 2016 •

edited

felixmosh commented Jan 9, 2017 •

edited

joshwiens commented Jan 19, 2017 •

edited

felixmosh Mar 6, 2017 •

edited