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

Read from textContent rather than innerHTML during rehydrate #3568

Merged
merged 2 commits into from
Sep 24, 2021

Conversation

keeganstreet
Copy link
Contributor

Resolves #3567

Sorry about the lack of tests. I tried adding the following test but it passes even without this change, so it's not particularly useful. I think that innerHTML behaves differently in actual browsers compared with the environment where the Jest tests are run (JSDOM?)

packages/styled-components/src/sheet/test/Rehydration.test.js

  it('supports > characters in selectors when style is inside another element', () => {
    document.body.innerHTML = `
      <svg>
        <style ${SC_ATTR} ${SC_ATTR_VERSION}="${SC_VERSION}">
          .b > * {}/*!sc*/
          ${SC_ATTR}.g22[id="idB"]{content:"nameB,"}/*!sc*/
        </style>
      </svg>
    `;
    const sheet = new StyleSheet({ isServer: true });
    rehydrateSheet(sheet);
    expect(sheet.getTag().getGroup(22)).toBe('.b > * {}/*!sc*/\n');
  });

@quantizor
Copy link
Contributor

I want to say we use innerHTML because textContent actually had buggy behavior in IE. @kitten do you remember?

@kitten
Copy link
Member

kitten commented Sep 20, 2021

@probablyup I don't quite remember anymore. We could just retest it. As long as we use textContent though, we should be fine. However, innerText is usually the one that's inconsistent and to avoid.

Copy link
Contributor

@quantizor quantizor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@quantizor
Copy link
Contributor

@keeganstreet Mind adding a quick changelog entry before I merge?

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@keeganstreet
Copy link
Contributor Author

@probablyup sure, done 👍

Would you like another PR to apply this to the legacy-v5 branch?

@quantizor
Copy link
Contributor

@probablyup sure, done 👍

Would you like another PR to apply this to the legacy-v5 branch?

Sure, go ahead.

@quantizor quantizor merged commit 97c6f70 into styled-components:main Sep 24, 2021
keeganstreet pushed a commit to keeganstreet/styled-components that referenced this pull request Sep 28, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@keeganstreet keeganstreet deleted the use-text-content branch September 28, 2021 11:36
quantizor pushed a commit that referenced this pull request Oct 19, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…hydrate) to v5 (#3591)

* Backport #3568 to v5

* Describe innerHTML to textContent change

Co-authored-by: Keegan Street <keegan.street@rea-group.com>
aplavsa pushed a commit to aplavsa/styled-components that referenced this pull request Mar 3, 2022
…nerHTML during rehydrate) to v5 (styled-components#3591)

* Backport styled-components#3568 to v5

* Describe innerHTML to textContent change

Co-authored-by: Keegan Street <keegan.street@rea-group.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ServerStyleSheet.interleaveWithNodeStream inserts style elements as children of SVG elements
3 participants