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

Aphrodite CSS with Byte Counting #332

Closed
bthecorgi opened this issue Jun 8, 2018 · 3 comments
Closed

Aphrodite CSS with Byte Counting #332

bthecorgi opened this issue Jun 8, 2018 · 3 comments

Comments

@bthecorgi
Copy link

bthecorgi commented Jun 8, 2018

Goal

To generate up to the specified bytes of CSS

Requirements

For AMP pages, CSS styling for a Pin is needed if it exists in the markup. We want to optimize the number of Pins on the page using the specified allowable bytes for AMP CSS as a constraint.

Aphrodite Overview

Aphrodite allows colocating css styles with javascript component. This makes it generally easier to maintain related code.

Below is a sample code snippet.

class App extends Component {
   render() {
       return <div>
           <span className={css(styles.red)}>
               This is red.
           </span>
       </div>;
   }
}

const styles = StyleSheet.create({
   red: {
       backgroundColor: 'red'
   },
});

Here are the main steps involving Aphrodite:

  1. css is called with the defined styles (ie. styles.red)
  2. generates the related CSS styles
  3. injects that generated style into a buffer (injectionBuffer is an string[]).
  4. Repeat steps 1 - 3 multiple times during render as needed by different components
  5. StyleSheetServer.renderStatic is called to render the final html markup
  6. flushToString is called to flush the content of the buffer

Proposal #1

Manually inspect injectionBuffer after each CSS is called to ensure that it is not over the byte limit, if so, remove the last entries in the injectionBuffer until the byte limit is satisfied.

Problems with this approach:

  • The injectionBuffer is not available for external inspection
  • Since injectionBuffer is managed by Aphrodite, altering it could have unintended consequences

Proposal #2

Built into Aphrodite the ability to accept CSS up to the specified limit. This proposal could look like one of the following:

Implementation A:

  1. Ability to specify a limit.
  2. css would fail if it has reached the limit.

Implementation B:

  1. Expose a new function for checking how many bytes a given style would use.
  2. Expose a new function to check how many bytes Aphrodite has already used.
  3. The user can then determine whether they want to continue adding more styles.
@bthecorgi bthecorgi changed the title Ability to get the stylesheet css byte size Aphrodite CSS with Byte Counting Jun 15, 2018
@kevinbarabash
Copy link
Contributor

What happens when the limit is reached and there's still styles that need to be generated?

@jlfwong
Copy link
Collaborator

jlfwong commented Apr 5, 2019

Author, but no longer maintainer here, so I do not have final say here, but IMO this is a really niche use case and does not belong in aphrodite core. For exactly the reason @kevinbarabash mentions, terminating generation of CSS in the middle of a React render doesn't make much sense to me.

If it's really important for you to fit under the limit, then IMO the less abstraction-breaking strategy would be to parse the resulting CSS and manipulate it as you see fit.

Or, (as you probably did, since this issue is pretty old now), run your own fork, which, if it serves your purposes, is great!

One of the design goals of aphrodite was to keep the interface small and avoid exposing details about internals, and I think termination of style generation mid-way through component generation would be contrary to both of those goals.

P.S. @willyau This PR description is super clear! Even though I disagree with merging this, I found it really easy to understand what problem you were facing and your proposals for addressing your problem. I hope that the solution you came up with served you well!

@kevinbarabash
Copy link
Contributor

I was going to suggest parsing the resulting CSS too. @jlfwong thanks for weighing in.

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

No branches or pull requests

3 participants