Injecting into the iframe

[dominykas]

While the issue title and my use case are kind of specific - I need to render the CSS into the different document than my current one (I'm creating an iframe and rendering inside of it), the problem is fundamental: simply calling require() should not have side effects.

Basically, this: (require("+JSON.stringify(__dirname)+"))(css); should not happen automatically - as a feature request, would it be possible to maybe disable this line based on some param, if having this breaking change is a no go?

Also, if I'm not mistaken, var css = inputString.replace(/\'/g, "\\\'").replace(/\"/g, "\\\"").replace(/\n/g, "\\\n"); would probably work as a simple var css=JSON.stringify(inputString);

[davidguttman]

Hmm... The problem is I'm not sure where we could pass in the option. I suppose if we were to change the API to something like:

styleAdder = require('./style.css')
styleAdder() // adds to the page
styleAdder.styles // could be the style string

It would be possible as one would have the discretion of invoking the function or not.

Not sure if you see another way of conditionally adding the style to the page.

[davidguttman]

As for inputString.replace(...).replace(...) becoming JSON.stringify(inputString), I just tried that and the test failed. If you can clean that up and get the test to pass, I'd be happy to take a PR.

[dominykas]

Yes, having something like a callable function returned, like you suggest, would solve the problem completely - but it's a breaking change - it's your call whether you want that or not :) (FWIW, I vote yes - IMHO it's a much better API).

As for conditional - I was thinking that maybe it can be done at compile time? Doesn't browserify pass some args? e.g. bool autoInject, with true as default for back compat, and if it is set to false - simply don't include the automatic require('cssify') in the output. This is pure speculation - not sure if that's doable at all - I still have to read up and understand the internals of browserify :)

[dominykas]

And sure - I'll try to PR the stringify bit once I catch a break this week.

[dominykas]

Meh. I would have done two separate PRs for stringify and for the --no-auto-inject, but Github did its thing (#8) and I don't want to mess with it :)

Anyways, running ./node_modules/.bin/browserify -t [./index --no-auto-inject] test/fixtures/basic/entry.js produces correct output (I think) - I'll go try this for real code now :)

[davidguttman]

Closed with 927ade8b44eb307fa07a25dc8e6181fa690294cd