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

Fix duplicated global style blocks #4

Merged
merged 1 commit into from Aug 6, 2020
Merged

Conversation

andy-hook
Copy link
Contributor

The upgrade to styled-components v5 was causing duplicate global style blocks to be rendered in the head. They seemed to originate from <Main/> in aragon/ui. For now I think it's best to pin it to v4.

@delfipolito this should resolve the issue we discussed with HMR reloads.

Copy link
Contributor

@sohkai sohkai left a comment

Choose a reason for hiding this comment

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

Looks good; cc @bpierre perhaps we should set the peer dependency range on aragonUI to be ^4 instead of >= 4?

package.json Outdated
@@ -41,7 +41,7 @@
"react": "^16.13.1",
"react-dom": "^16.13.1",
"react-spring": "^8.0.1",
"styled-components": "^5.1.1"
"styled-components": "4.4.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we can still leave this with a ^ for the 4.x range

@bpierre
Copy link
Contributor

bpierre commented Aug 6, 2020

Looks good; cc @bpierre perhaps we should set the peer dependency range on aragonUI to be ^4 instead of >= 4?

We could do that yes, but I suspect it will break apps already already using aragonUI with styled-components v5.

I would like to understand the issue as aragonUI should work with v5. @andy-hook could you tell me how to reproduce? It seems to work fine with the devbox or the client. 🤔

Edit: in any case, it’s probably a good idea to use >= 4 < 6 in case v6 breaks compatibility.

@andy-hook
Copy link
Contributor Author

andy-hook commented Aug 6, 2020

Looks good; cc @bpierre perhaps we should set the peer dependency range on aragonUI to be ^4 instead of >= 4?

We could do that yes, but I suspect it will break apps already already using aragonUI with styled-components v5.

I would like to understand the issue as aragonUI should work with v5. @andy-hook could you tell me how to reproduce? It seems to work fine with the devbox or the client. 🤔

Edit: in any case, it’s probably a good idea to use >= 4 < 6 in case v6 breaks compatibility.

I think I've identified the issue with fresh eyes today. See styled-components/styled-components#3008

Edit: You can repro in the client by wrapping the root in <React.StrictMode> with v5 styled-components

Knowing this, a better fix in this project might be to nest <React.StrictMode/> inside of <Main/> and keep v5 for the performance improvements. What do you think?

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.

None yet

4 participants