Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

why styled-component breaks in IE11 when used by angular? #16215

Closed
2 tasks
adamchenwei opened this issue Sep 5, 2017 · 6 comments
Closed
2 tasks

why styled-component breaks in IE11 when used by angular? #16215

adamchenwei opened this issue Sep 5, 2017 · 6 comments

Comments

@adamchenwei
Copy link

adamchenwei commented Sep 5, 2017

I'm submitting a ...

  • [x ] bug report
  • feature request
  • other (Please do not submit support requests here (see above))

Current behavior:
Style gone missing when styled-components is used inside angularjs application (even if the styled component was applied in React if its loaded into anuglarjs, it breaks anyway. Run in React alone has no issue whatsoever.

Expected / new behavior:
Angular should be able to render styled components style correctly.

Minimal reproduction of the problem with instructions:
https://github.com/dfrankland/styled-components-angular-ie-11-conflict

AngularJS version: 1.x.y (latest)

Browser:
IE 11

Anything else:

@adamchenwei
Copy link
Author

adamchenwei commented Sep 5, 2017

I would love to know even just how to approach this issue (which file maybe suspect? where would be the best entry point to start the debugging? etc) .... wonder how would someone debugging this anyway?

@gkalpak
Copy link
Member

gkalpak commented Sep 5, 2017

I did some debugging and here is what I found:

  • styled.button creates a new <style> element in <head>, creates and empty textNode inside it and keeps a reference to that textNode. (Actually, it creates two empty textNodes - this will become relevant later 😃).
  • When the element is rendered (by React), it appends the necessary CSS rules to the aforementioned textNode (the second one).
  • In IE11, by the time the component in rendered, the second textNode is no longer a chid of the <style> element.

This happens, because in IE11 we merge consecutive textNodes in order to work around another IE11 bug (see #11781 and #14924 for more details):

angular.js/src/ng/compile.js

Lines 2000 to 2002 in ecc09a4

if (msie === 11) {
mergeConsecutiveTextNodes(nodeList, i, notLiveList);
}

I am afraid there is not much we can do (without breaking other usecases).

Depending on your requirements, there are some ways you could work around this:

  1. Bootstrap your app on document.body (most apps don't really need to be bootstrapped above body).
  2. Do not create your Button element (which creates the <style> element) until after angular has bootstrapped.

@supereminem
Copy link

U can do anything u set your mind to

@adamchenwei
Copy link
Author

adamchenwei commented Sep 6, 2017

@gkalpak Thanks for the debugging work! Just few quick thing

  1. How you reach the line of code mergeConsecutiveTextNodes? Is that because you kinda already know?
  2. severity: inconvenient - I guess it not just inconvenient, but rather very inconvenient to a level that prevents the company to properly utilize styled-components for customers in IE11 (which account for a good portion of the company's customers), ever. I mean, the company decides to slowly convert all angular to react app, but seems this issue just help to boost the process (a bit) faster....
  3. Is won't fix mean, ... we are never ever ever going to fix it, EVER?
  4. Thanks for the workaround though! seems the appending to document.body is working fo the demo I uploaded.

Thanks !!!!

@gkalpak
Copy link
Member

gkalpak commented Sep 6, 2017

How you reach the line of code mergeConsecutiveTextNodes? Is that because you kinda already know?

Pretty much yes. I mean, I stepped through the styled.button code with the debugger (because I had no idea what it does) and I knew (or suspected) it is something that happens when we run $compile($rootElement)($rootScope) inside AngularJS' bootstrap.
I would step through that, but knowing how styled.button works and what $compile() does, it was obvious.

severity: inconvenient - I guess it not just inconvenient, but rather very inconvenient

This essentially means, that you can work around it. So, it is inconvenient, but doesn't stop you from having a working app.

Is won't fix mean, ... we are never ever ever going to fix it, EVER?

Never say never, but yeah, that's the general idea. Closing as won't fix was my suggestion, but we haven't decided yet.

@jbedard, had an interesting idea, which might work:
When we merge consecutive text nodes in IE, instead of removing the extra ones, leave them as empty text nodes.

@adamchenwei, if you want to take a stub at it and see if any test breaks (and whether it works in your demo), feel free to go ahead 😁

@gkalpak gkalpak added this to the Backlog milestone Sep 11, 2017
@gkalpak
Copy link
Member

gkalpak commented Sep 12, 2017

We discussed this and decided that it is not worth trying to fix, given that:

  • This is a quite edgy case.
  • There are work-arounds available.
  • There is no "right" way to fix it (i.e. whatever work-around we put in place could break some other edge case).

It is unfortunate that we have to work-around IE11's bug in the first place, but it is what it is 😃

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

No branches or pull requests

3 participants