Skip to content
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

Angular.js causes styled-components to break on IE 11 #657

Closed
dfrankland opened this issue Apr 6, 2017 · 17 comments
Closed

Angular.js causes styled-components to break on IE 11 #657

dfrankland opened this issue Apr 6, 2017 · 17 comments

Comments

@dfrankland
Copy link

dfrankland commented Apr 6, 2017

Version

1.4.4 (maybe a lot older too)

Reproduction

Webpackbin doesn't work on IE 11... So here's a repo!
https://github.com/dfrankland/styled-components-angular-ie-11-conflict

Steps to reproduce

Follow steps in linked repo.

Expected Behavior

Work like normal.

Actual Behavior

No CSS rules are applied to the style element.

Why?

Angular will recompile all the individual DOM elements including those in the head. Normally, on Chrome and other browsers this isn't an issue, but on IE 11 this causes text nodes to lose their parentNode. This could occur due to anything though, it doesn't have to be Angular. The more longwinded explanation:

  1. Style elements and text nodes get inserted in ComponentStyle

  2. Afterwards, anything could mutate the DOM, such as Angular, causing issues later

  3. A styled component is ready to render so generateAndInjectStyles inside of ComponentStyle is called

  4. appendRule inside the Glamor sheet tries to mutate a text node, but it has already been removed from the DOM, thus no styles are applied to the style tags created in step 1

Fix

I propose to move the appending of text nodes to the style element DOM only once the component is ready to render, rather than doing it preemptively.

https://github.com/styled-components/styled-components/blob/master/src/vendor/glamor/sheet.js#L125

const textNode = document.createTextNode(rule)
- last(this.tags).appendChild(textNode)
- insertedRule = { textNode, appendRule: newCss => textNode.appendData(newCss)}
+ insertedRule = {
+   textNode,
+   appendRule: newCss => textNode.appendData(newCss),
+   appendNode: () => last(this.tags).appendChild(textNode),
+ };

https://github.com/styled-components/styled-components/blob/master/src/models/ComponentStyle.js#L39

const selector = nameGenerator(hash)
inserted[hash] = selector
const root = parse(`.${selector} { ${flatCSS} }`)
postcssNested(root)
autoprefix(root)
this.insertedRule.appendRule(root.toResult().css)
+ this.insertedRule.appendNode()
@k15a k15a added the bug 🐛 label Apr 6, 2017
@garretruh
Copy link

garretruh commented Apr 6, 2017

Seeing the same issue with styled-components@1.3.0. Any components on a page with Angular (v1.6.4 in this case) that are rendered after Angular has initialized don't get their styles injected. The included patch does appear to resolve the issue.

@kitten
Copy link
Member

kitten commented Apr 6, 2017

We don't want to modify the vendored glamor module, however we can still defer the injection of the stylesheet. It's fairly simple, but is that stuff we want, @mxstbr @geelen?

@mxstbr
Copy link
Member

mxstbr commented Apr 7, 2017

This is @geelen expertise, there's a reason why we do it exactly the way we do.

I don't want to change that part of the code without having benchmarked it extensively to make sure we don't get less performant and I need @geelen to confirm this'll work so we avoid subtle ordering bugs like #1. (which are a pain)

@kitten
Copy link
Member

kitten commented Apr 8, 2017

@mxstbr Ah, yep, you're right. If we defer injection to the rendering, then the order of CSS is going to change to the order of rendering, not the order of imports. Looks like this is not sth we'll fix anytime soon :/

@dfrankland
Copy link
Author

dfrankland commented Apr 8, 2017

@mxstbr @philpl this is definitely true. Maybe the best option in the meantime is just to do a check for if the textNode has lost its parentNode and throw warnings 🤔

For example:

const selector = nameGenerator(hash)
inserted[hash] = selector
const root = parse(`.${selector} { ${flatCSS} }`)
postcssNested(root)
autoprefix(root)
+ if (this.textNode && !this.textNode.parentNode) {
+   console.warn('There may be an issue with the styles, the stylesheet DOM has been mutated.')
+ }
this.insertedRule.appendRule(root.toResult().css)

@kitten
Copy link
Member

kitten commented May 22, 2017

It's not really sth that is a bug on our end, or sth that we can affect or fix unfortunately

@jasonalmaturner
Copy link

I've had this same problem with using styled-components in the angular/react app I'm working on. I found this snippet by @dfrankland that fixed my problem.

angular.element(document).ready(() => {
  const bootStrapElement = (
    // BAD! Recompiles everything causing IE 11 to not work
    document

    // GOOD! Does not mess with the <head>
    // document.body
  );
  angular.bootstrap(bootStrapElement, ['blah'], {});
});

https://github.com/dfrankland/styled-components-angular-ie-11-conflict/blob/master/src/main.js

@HashemKhalifa
Copy link

@jasonalmaturner Would u tell me what u changed?!

@mxstbr
Copy link
Member

mxstbr commented Aug 28, 2017

- angular.bootstrap(document, ['blah'], {});
+ angular.bootstrap(document.body, ['blah'], {});

@HashemKhalifa
Copy link

actually, I'm trying to fix my issue using this solution but I'm using react2angular to handle react components separately what I have to follow?!

@dfrankland
Copy link
Author

dfrankland commented Aug 28, 2017

@HashemKhalifa, using react2angular doesn't really affect the way the fix is implemented.

Following the react2angular example, you only need to add one line:

import { react2angular } from 'react2angular'

angular
  .module('myModule', [])
  .component('myComponent', react2angular(MyComponent, ['fooBar', 'baz']))
+ .element(document).ready(() => angular.bootstrap(document.body, ['myModule'], {}));

If you need any other help, you may want to open an issue for react2angular.

@HashemKhalifa
Copy link

@dfrankland Thanks for your reply, .element here refer to what?
this is my example but it didn't work.
.component('hotelSearchMapComponent', react2angular(hotelSearchMapComponent, ['lat', 'lng', 'zoom', 'hotels', 'symbol', 'rate', 'from', 'to', 'onSelectHotel'])) .element(document).ready(() => angular.bootstrap(document.body, [MODULE_NAME], {}))

@HashemKhalifa
Copy link

getting these errors
screen shot 2017-08-29 at 12 01 27 pm
screen shot 2017-08-29 at 12 01 36 pm

@kitten
Copy link
Member

kitten commented Aug 29, 2017

I think at this point you should consider opening an issue on their repo instead 😉

@adamchenwei
Copy link

adamchenwei commented Aug 31, 2017

Hey everyone, someone even opened a repo to demostrate the bug, that when styled-component run with AngularJS in IE11, it breaks.
https://github.com/dfrankland/styled-components-angular-ie-11-conflict

Any fixes atm? really need it, super sad it happens...

@kitten
Copy link
Member

kitten commented Aug 31, 2017

@adamchenwei To quote myself:

It's not really sth that is a bug on our end, or sth that we can affect or fix unfortunately

I don't really have time to look at it in detail, but the proposed fix confirmed it for me. If someone could explain why it happens and it turns out that it's a problem with our codebase, then we'd be able to actually fix it.

@byrekt
Copy link

byrekt commented Sep 18, 2017

Just a heads-up. If you use auto-bootstrapping with ng-app on the html tag of your index.html, you can try moving ng-app to the body tag. That resolved it for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants