Add insertInto option #135

ahstro · 2016-07-13T16:06:39Z

Add another option called insertInto - which allows specifying a CSS selector - e.g. #styles - to be used as an argument to document.querySelector to find an element into which to insert the generated <style>-tags. Will default to the <head>-tag if undefined.

This was created because I wanted to insert <style> tags into a ShadowRoot (like in #131), but this is not a specific solution for that, but does solve it by allowing people to first create a ShadowRoot and then target it using insertInto and the appropriate selector.

Not att all sure if this is wanted, but it exists, so there ya go :)
Also, I replaced the getHeadElement function with a custom memoized function rather than add the key feature to the existing memoizing function, not sure if that was the better choice, but it felt like it.

Closes #131

geekuillaume · 2017-02-13T10:49:25Z

Is there any news on this PR ?
@ahstro you need to accept the CLA to merge this.

Thanks for your work !

ahstro · 2017-02-14T08:45:18Z

@geekuillaume The "Details" link dosn't work, says "There is no CLA to sign for webpack/style-loader". Same is true when I change webpack in the URL to webpack-contrib.

rustydb · 2017-02-22T14:43:17Z

Can anyone sign this? Really am looking forward to this being merged in.

ahstro · 2017-02-22T15:09:02Z

Signing the CLA worked now, so if there's nothing I need to change, this should be good to merge now.

DanielBadan · 2017-02-23T08:53:56Z

So is there anyone reviewing PRs?

michael-ciniawsky · 2017-03-06T17:28:45Z

addStyles.js

@@ -13,8 +13,16 @@ var stylesInDom = {},
 	isOldIE = memoize(function() {
 		return /msie [6-9]\b/.test(window.navigator.userAgent.toLowerCase());
 	}),
-	getHeadElement = memoize(function () {
-		return document.head || document.getElementsByTagName("head")[0];
+	getStyleTarget = (function(fn) {


getStyleTarget => getElement/getStyle

bebraw

Would it be possible to add tests for this?

ekulabuhov · 2017-03-06T18:31:35Z

This is very similar to #65. Whichever is merged first most likely to close the other too.

ahstro · 2017-03-06T20:34:16Z

@bebraw I'm looking at testing, but the existing test utils seem to be built around this feature not existing, by comparing only the contents of the <head> tag with the expected result. The solution I came up with is comparing the entire jsdom document source with a string gotten from a new method on utils that inserts the expected style output at some index in the jsdomHtml, but that might be a really bad way of doing things and I'm tired, so I'm not going to work more on that now. Hope you or someone approves or, preferably, has a better idea soon. :)

bebraw · 2017-03-06T20:39:40Z

@ekulabuhov Any idea on what would be a good way to test the change?

ekulabuhov · 2017-03-07T00:39:22Z

@ahstro I think you might need to replace that document.head reference with the querySelector. Add path as the last parameter to runCompilerTest and default it to head.

Come to think about it, I'm not sure if querySelector can query Shadow DOM.

Ping me if you have any troubles adding this.

ahstro · 2017-03-07T08:56:15Z

@ekulabuhov We're using my fork for work to query a ShadowDOM, so it works, you just need some fancy query.

I don't have access to the computer where I started work on testing yesterday, so I can't fix it right now, but I'll try to get to it later today.

michael-ciniawsky · 2017-03-13T17:52:07Z

For v1.0.0 I'm currently drafting a few refactors for discussion (will open a detailed issue later :D) and one of the things I would like to change is e.g

Merge insertAt/insertTo/... => selector: 'head' || 'body' || '#el' || '.el' (use querySelector + fallback, default selector is 'head') like in this PR

michael-ciniawsky · 2017-03-13T17:53:48Z

README.md

@@ -47,9 +47,12 @@ Note: Behavior is undefined when `unuse`/`unref` is called more often than `use`

 ### Options

+#### `insertInto`


Please put it directly below the insertAt section

michael-ciniawsky · 2017-03-13T18:00:15Z

addStyles.js

+			return memo[selector]
+		};
+	})(function (styleTarget) {
+		return document.querySelector(styleTarget)


We need a fallback here, I guess 😛 ?

Pseudo Fallback

if (!document.querySelector) { if (selector.match('#')) return document.getElementById(selector) if (selector.match('.')) return document.getElementByClassName(selector) return document.getElementByTagName(selector) } else { return document.querySelector(selector) }

Do we really? According to Can I Use, the support for querySelector in newer browsers is better than that of getElementsByClassName, the latter not having any support for IE8 while the former has partial support.

Forget what I said then :D

michael-ciniawsky · 2017-03-13T18:08:20Z

Also please take a look into #126 adding it to useable aswell 🙃

ekulabuhov · 2017-03-13T20:34:00Z

@michael-ciniawsky did you mean #126 in your last comment? 😀

I like the idea of merging insertAt and insertTo 👍

ahstro · 2017-03-13T21:38:01Z

I do like the idea of merging them, but if I understood it correctly, that's out of scope for this PR, right?

ahstro · 2017-03-13T23:14:53Z

@michael-ciniawsky No problem :) Did you see the comment I left on your line comment regarding the fallback?

michael-ciniawsky

@ahstro Finally a test would be 💯

bebraw

Adding tests would be ideal.

michael-ciniawsky · 2017-03-17T00:30:05Z

Closes #44

joshwiens

Yes to the feature & it's implementation.

It won't merge until it's properly tested

michael-ciniawsky · 2017-03-17T01:01:48Z

Closes #131

Add another option - called `insertInto` - which allows specifying a CSS selector - e.g. `#styles` - to be used as an argument to `document.querySelector` to find an element into which to insert the generated `<style>`-tags. Will default to the `<head>`-tag if undefined.

ahstro · 2017-03-17T17:21:08Z

Okay, added tests :) I kept it a separate commit so I can easily change anything if it's not satisfactory; let me know if I should squash them.

ahstro · 2017-03-17T17:27:24Z

Changed default parameter syntax because Travis complained

michael-ciniawsky

@ahstro Thx

jinyang1994 · 2018-11-21T10:11:01Z

How to use this feature? I want insert into multiple iframe.

ahstro force-pushed the master branch from 28d8144 to b490d22 Compare July 22, 2016 11:47

ahstro force-pushed the master branch from b490d22 to f4ec82a Compare March 1, 2017 16:12

michael-ciniawsky requested review from bebraw, ekulabuhov, michael-ciniawsky and joshwiens March 6, 2017 17:26

michael-ciniawsky requested changes Mar 6, 2017

View reviewed changes

bebraw reviewed Mar 6, 2017

View reviewed changes

ahstro force-pushed the master branch from f4ec82a to 1d092a0 Compare March 6, 2017 20:03

michael-ciniawsky requested changes Mar 13, 2017

View reviewed changes

michael-ciniawsky added type: Feature pr: refactor pr_action: Review labels Mar 13, 2017

ekulabuhov mentioned this pull request Mar 13, 2017

[Enhancement] Add parentElement parameter to use/ref method #126

Closed

ekulabuhov mentioned this pull request Mar 13, 2017

Access styles at runtime #152

Closed

michael-ciniawsky approved these changes Mar 14, 2017

View reviewed changes

michael-ciniawsky added pr: tests and removed pr: refactor labels Mar 14, 2017

bebraw reviewed Mar 14, 2017

View reviewed changes

joshwiens force-pushed the master branch from afd5db9 to 2630c0f Compare March 15, 2017 04:22

michael-ciniawsky mentioned this pull request Mar 17, 2017

Passing document to .use #44

Closed

joshwiens requested changes Mar 17, 2017

View reviewed changes

michael-ciniawsky removed the pr_action: Review label Mar 17, 2017

michael-ciniawsky mentioned this pull request Mar 17, 2017

insertAt shadow root? #131

Closed

ahstro force-pushed the master branch from f336c95 to 5d6a98d Compare March 17, 2017 17:19

bebraw approved these changes Mar 17, 2017

View reviewed changes

Test insertInto option

c5ca476

ahstro force-pushed the master branch from 5d6a98d to c5ca476 Compare March 17, 2017 17:26

michael-ciniawsky added pr: accepted and removed pr: tests labels Mar 17, 2017

michael-ciniawsky approved these changes Mar 17, 2017

View reviewed changes

michael-ciniawsky merged commit 6636868 into webpack-contrib:master Mar 17, 2017

michael-ciniawsky removed type: Feature pr: accepted labels Mar 17, 2017

michael-ciniawsky mentioned this pull request Mar 26, 2017

Allow to specify element into which to inject Style. #65

Closed

cdbkr mentioned this pull request Jul 10, 2017

insertInto and ShadowDom #256

Closed

jinyang1994 mentioned this pull request Nov 21, 2018

How to to inject the style into a virtual Iframe #351

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add insertInto option #135

Add insertInto option #135

ahstro commented Jul 13, 2016

geekuillaume commented Feb 13, 2017

ahstro commented Feb 14, 2017

rustydb commented Feb 22, 2017 •

edited

Loading

ahstro commented Feb 22, 2017

DanielBadan commented Feb 23, 2017

michael-ciniawsky Mar 6, 2017

bebraw left a comment

ekulabuhov commented Mar 6, 2017 •

edited

Loading

ahstro commented Mar 6, 2017

bebraw commented Mar 6, 2017

ekulabuhov commented Mar 7, 2017

ahstro commented Mar 7, 2017

michael-ciniawsky commented Mar 13, 2017 •

edited

Loading

michael-ciniawsky Mar 13, 2017

ahstro Mar 13, 2017

michael-ciniawsky Mar 13, 2017

ahstro Mar 13, 2017

michael-ciniawsky Mar 13, 2017 •

edited

Loading

michael-ciniawsky commented Mar 13, 2017 •

edited

Loading

ekulabuhov commented Mar 13, 2017

ahstro commented Mar 13, 2017

ahstro commented Mar 13, 2017

michael-ciniawsky left a comment

bebraw left a comment

michael-ciniawsky commented Mar 17, 2017

joshwiens left a comment

michael-ciniawsky commented Mar 17, 2017

ahstro commented Mar 17, 2017

ahstro commented Mar 17, 2017

michael-ciniawsky left a comment

jinyang1994 commented Nov 21, 2018

		@@ -47,9 +47,12 @@ Note: Behavior is undefined when `unuse`/`unref` is called more often than `use`

		### Options

		#### `insertInto`

Add insertInto option #135

Add insertInto option #135

Conversation

ahstro commented Jul 13, 2016

geekuillaume commented Feb 13, 2017

ahstro commented Feb 14, 2017

rustydb commented Feb 22, 2017 • edited Loading

ahstro commented Feb 22, 2017

DanielBadan commented Feb 23, 2017

michael-ciniawsky Mar 6, 2017

Choose a reason for hiding this comment

bebraw left a comment

Choose a reason for hiding this comment

ekulabuhov commented Mar 6, 2017 • edited Loading

ahstro commented Mar 6, 2017

bebraw commented Mar 6, 2017

ekulabuhov commented Mar 7, 2017

ahstro commented Mar 7, 2017

michael-ciniawsky commented Mar 13, 2017 • edited Loading

michael-ciniawsky Mar 13, 2017

Choose a reason for hiding this comment

ahstro Mar 13, 2017

Choose a reason for hiding this comment

michael-ciniawsky Mar 13, 2017

Choose a reason for hiding this comment

ahstro Mar 13, 2017

Choose a reason for hiding this comment

michael-ciniawsky Mar 13, 2017 • edited Loading

Choose a reason for hiding this comment

michael-ciniawsky commented Mar 13, 2017 • edited Loading

ekulabuhov commented Mar 13, 2017

ahstro commented Mar 13, 2017

ahstro commented Mar 13, 2017

michael-ciniawsky left a comment

Choose a reason for hiding this comment

bebraw left a comment

Choose a reason for hiding this comment

michael-ciniawsky commented Mar 17, 2017

joshwiens left a comment

Choose a reason for hiding this comment

michael-ciniawsky commented Mar 17, 2017

ahstro commented Mar 17, 2017

ahstro commented Mar 17, 2017

michael-ciniawsky left a comment

Choose a reason for hiding this comment

jinyang1994 commented Nov 21, 2018

rustydb commented Feb 22, 2017 •

edited

Loading

ekulabuhov commented Mar 6, 2017 •

edited

Loading

michael-ciniawsky commented Mar 13, 2017 •

edited

Loading

michael-ciniawsky Mar 13, 2017 •

edited

Loading

michael-ciniawsky commented Mar 13, 2017 •

edited

Loading