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

StylesheetManager does not render all styles in iframes #2973

Closed
dcastil opened this issue Jan 16, 2020 · 9 comments · Fixed by #3159
Closed

StylesheetManager does not render all styles in iframes #2973

dcastil opened this issue Jan 16, 2020 · 9 comments · Fixed by #3159
Labels

Comments

@dcastil
Copy link

dcastil commented Jan 16, 2020

Environment

npx envinfo --system --binaries --npmPackages styled-components,babel-plugin-styled-components --markdown --clipboard

System:

  • OS: macOS 10.15.2
  • CPU: (8) x64 Intel(R) Core(TM) i7-8559U CPU @ 2.70GHz
  • Memory: 385.48 MB / 16.00 GB
  • Shell: 5.7.1 - /bin/zsh

Binaries:

  • Node: 12.14.1 - /usr/local/bin/node
  • Yarn: 1.21.1 - /usr/local/bin/yarn
  • npm: 6.13.6 - /usr/local/bin/npm

Reproduction

styled-components v4.3.2: https://primer.style/doctocat/components/do-dont
styled-components v5.0.0: https://doctocat-ajl99nptm.now.sh/doctocat/components/do-dont

Changes made between both deployments: primer/doctocat#108
Source code where StylesheetManager is used: https://github.com/dcastil/doctocat/blob/styled-components-v5/theme/src/components/frame.js#L17

Steps to reproduce

  1. Clone https://github.com/dcastil/doctocat
  2. Switch to branch styled-components-v5
  3. Run yarn && yarn start in repo root
  4. Navigate to http://localhost:8000/components/do-dont in browser

Expected Behavior

CSS is rendered into head of each iframe.

Actual Behavior

Some CSS is rendered and some not. I couldn't find a pattern in which CSS is rendered and which isn't unfortunately. Also if I delete the first iframe in https://github.com/dcastil/doctocat/blob/styled-components-v5/docs/content/components/do-dont.mdx, other CSS is being rendered in the second one. So the StylesheetManager instances seem to interfere with each other.

@dcastil dcastil changed the title SrylesheetManager does not render all styles in iframes StylesheetManager does not render all styles in iframes Jan 16, 2020
@kitten
Copy link
Member

kitten commented Jan 16, 2020

@probablyup looks like we’re not iFrame safe anymore, since even if a new target is passed the names record is shared and hence a rule group can only be inserted once

@quantizor
Copy link
Contributor

It looks like we already have a test for this...

it('should inject styles into two parallel contexts', async () => {
const Title = styled.h1`
color: palevioletred;
`;
// Injects the stylesheet into the document available via context
const SheetInjector = ({ children, target }) => (
<StyleSheetManager target={target}>{children}</StyleSheetManager>
);
class Child extends React.Component {
componentDidMount() {
const styles = this.props.document.querySelector('style').textContent;
expect(styles.includes(`palevioletred`)).toEqual(true);
this.props.resolve();
}
render() {
return <Title />;
}
}
const div = document.body.appendChild(document.createElement('div'));
let promiseB;
const promiseA = new Promise((resolveA, reject) => {
promiseB = new Promise(resolveB => {
try {
// Render two iframes. each iframe should have the styles for the child injected into their head
render(
<div>
<Frame>
<FrameContextConsumer>
{({ document }) => (
<SheetInjector target={document.head}>
<Child document={document} resolve={resolveA} />
</SheetInjector>
)}
</FrameContextConsumer>
</Frame>
<Frame>
<FrameContextConsumer>
{({ document }) => (
<SheetInjector target={document.head}>
<Child document={document} resolve={resolveB} />
</SheetInjector>
)}
</FrameContextConsumer>
</Frame>
</div>,
div
);
} catch (e) {
reject(e);
div.parentElement.removeChild(div);
}
});
});
await Promise.all([promiseA, promiseB]);
div.parentElement.removeChild(div);
});

@nikparo
Copy link

nikparo commented Feb 6, 2020

... I couldn't find a pattern in which CSS is rendered and which isn't unfortunately. ...

I believe we have run into this issue as well when trying to upgrade to version 5. It seems that whichever frame or window happens to render a component first is given the styles for that component.

@kitten
Copy link
Member

kitten commented Feb 6, 2020

Yes, I think we need to make sure that the name cache isn’t copied anymore but we forgot to send a patch for this in v5.0.1. I’ll open a quick PR for this 👍

@seeden
Copy link

seeden commented Feb 15, 2020

Pattern is: everything witch is already used in "main" head is not used again in StyleSheetManager.target

@sanex3339
Copy link

Hi. Any news? The same problem for us

@marioacc
Copy link

Hey guys,

My team is currently using styled-components 4.4.1 and we are experiencing a similar issue as well

@eramdam
Copy link
Contributor

eramdam commented May 30, 2020

Having the same issue here with child windows instead of iframes 😅

@KagamiChan
Copy link

KagamiChan commented Jun 14, 2020

Hi @kitten, could you please help provide some suggestions on fixing this? Do you think #3159 is a good candidate for fix, or you'd prefer adding a new props to stylesheet manager?

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

Successfully merging a pull request may close this issue.

9 participants