Skip to content

Add Stylesheet class that proxies methods to two stylesheets - #440

Merged
mxstbr merged 4 commits into
styled-components:masterfrom
xcoderzach:second-pass-global-styles
Feb 4, 2017
Merged

Add Stylesheet class that proxies methods to two stylesheets#440
mxstbr merged 4 commits into
styled-components:masterfrom
xcoderzach:second-pass-global-styles

Conversation

@xcoderzach

Copy link
Copy Markdown
Contributor

Adding a separate global stylesheet broke server side rendering,
because stylesheet.rules only returned component styles. We now
have a single stylesheet class, that inserts into the specified
sheet, and proxies all other methods to both sheets.

Adding a separate global stylesheet broke server side rendering,
because stylesheet.rules only returned component styles.  We now
have a single stylesheet class, that inserts into the specified
sheet, and proxies all other methods to both sheets.
@mxstbr-bot

mxstbr-bot commented Jan 31, 2017

Copy link
Copy Markdown
Warnings
⚠️ There are library changes, but not tests. That's OK as long as you're refactoring existing code

Generated by 🚫 dangerJS

@mxstbr

mxstbr commented Jan 31, 2017

Copy link
Copy Markdown
Member

Nice one! Should we ship this in v2 too?

@xcoderzach

Copy link
Copy Markdown
Contributor Author

Shipping this in v2 will require a few changes to how we do this, since the stylesheet module is quite a bit different in v2

@xcoderzach

Copy link
Copy Markdown
Contributor Author

I can make a separate pr into v2 if you'd like.

@mxstbr

mxstbr commented Jan 31, 2017

Copy link
Copy Markdown
Member

I can make a separate pr into v2 if you'd like.

I fear that this'll mess us up when we merge master into v2. Any ideas how we could avoid that?

@xcoderzach

Copy link
Copy Markdown
Contributor Author

My PR into v2 can be two things:

  1. Merge master into v2.
  2. Make it actually work in v2.

@mxstbr

mxstbr commented Jan 31, 2017

Copy link
Copy Markdown
Member

Sounds good!

@mxstbr

mxstbr commented Jan 31, 2017

Copy link
Copy Markdown
Member

Need to fix CI to land this though 😉

@xcoderzach

Copy link
Copy Markdown
Contributor Author

oops, on it.

@mxstbr mxstbr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks so much! Will publish as v1.4.2, looking forward to the PR against v2

@xcoderzach

Copy link
Copy Markdown
Contributor Author

Cool, I'll make the v2 PR as soon as this is merged.

@diegohaz

Copy link
Copy Markdown
Member

Amazing, @xcoderzach Thanks

@mxstbr
mxstbr merged commit 46b58c4 into styled-components:master Feb 4, 2017
@mxstbr

mxstbr commented Feb 4, 2017

Copy link
Copy Markdown
Member

Sorry, totally forgot to merge this. Will ship now!

@mxstbr

mxstbr commented Feb 7, 2017

Copy link
Copy Markdown
Member

Ping @xcoderzach, don't want to forget about this PR. Any news?

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.

5 participants