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

discussion: add cssGlobal #148

Closed
wants to merge 1 commit into from
Closed

discussion: add cssGlobal #148

wants to merge 1 commit into from

Conversation

mattkrick
Copy link

Aphrodite is a lightweight package for modular styles. That's why we love it.
But some things aren't modular:

  • fonts
  • global resets/normalizers
  • 3rd party css overrides

The current best practice is to use something other than aphrodite for these cases (except in the case of fonts, where aphrodite has kinda hacky logic).

With a very small amount of code, we can support globals (including SSR).

Globals are actually very easy:

  • They only need to be called once (no dependency on context/state/props)
  • Aphrodite doesn't need to worry about cascade order because globals should be written as specific as necessary
  • It allows aphrodite to get rid of that messy font handling (will adjust this PR if you like where this is going)

Why a 3rd party package doesn't work:

  • isBuffering is private, which means the user must pass injectStyleOnce to their function (otherwise you'd have 2 discrete flush cycles for the SSR). This is doable: see hepha. However, it'd be so much cleaner to include it in aphrodite as I'm sure everyone has a custom font, or {margin: 0} body tag that they'd like to load properly 😄

@@ -77,9 +77,22 @@ const css = (...styleDefinitions) => {
return injectAndGetClassName(useImportant, styleDefinitions);
};

const injectedGlobals = new WeakSet();
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless I'm mistaken, this requires a polyfill. This project's probably not ready to drop Internet Explorer :)

@mattkrick
Copy link
Author

Yeah, Edge+ (http://kangax.github.io/compat-table/es6/#test-WeakSet).
A weakset is used on the off chance that someone creates their globalStyles inside the render loop, as that'd create a memory leak.
If people use it like they should a Set works just fine.

@jlfwong
Copy link
Collaborator

jlfwong commented Oct 4, 2016

Hi @mattkrick,

Thanks for the thoughtful discussion!

Having support for global styles should be possible once #95 lands, and I'd prefer not to provide multiple mechanisms for it.

This PR doesn't work for SSR since it doesn't reset the injectGlobals anywhere. That aside though, I think this use case should be covered by #95, so I'm going to close this.

Let me know if I'm misunderstanding

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.

3 participants