blakeembrey / free-style

Make CSS easier and more maintainable by using JavaScript
MIT License
707 stars 29 forks source link

Suggestion: Post-processing that doesn't alter the style hash #35

Closed kimamula closed 6 years ago

kimamula commented 8 years ago

Currently if one wants to use free-style with CSS processors such as PostCSS, the CSS processing must be executed both on the server and the client sides. In my project I introduced free-style with PostCSS autoprefixer, which made the client-side JS bundle ~2 MBs larger in size, mainly due to caniuse-db which is depended on by autoprefixer.

By eliminating client-side CSS processing, we can

  1. keep our client side JS bundle small
  2. reduce performance overhead on the client side

I think this can be achieved by separating the hash calcuration process from the CSS output process. The proposed solution is to introduce a function to add processors to a FreeStyle instance:

type Processor = (userStyles: UserStyles) => UserStyles;

class FreeStyle {
  // ... Other stuff

  // Probably it'd be better to add processors at constructor
  addProcessors(processors: Processor | Processor[]): void;
}

When registerStyle is called after calling addProcessors, the returned hash is calculated based on its argument (as usual), while the output CSS is processed by the added processors. To use this feature, we call addProcessors somewhere on the server-side code and inject that FreeStyle instance to the component classes, while on the client side we inject an instance with no added processors. This way we can avoid client-side processing of CSS while sharing the same hashes both on the server and the client sides.

The main drawback would be that this disables CSS to be dynamically created on the client side. Therefore, react-free-style, for example, would not be able to depend on this feature.

blakeembrey commented 8 years ago

I think I'd like to see a formal proposal to understand better, but let me put some initial thoughts down.

A lot of people I've seen actually do use this as a dynamic injection of CSS - see stylin, easy-style. I think it'll be continued to be used like that. For react-free-style, I assume you're referring to the one feature (not the feature where you can compose styles beforehand).

I'd love to make this work for you, let's just figure out the best way for that to happen 😄 Does it make sense that the hashes need to remain the same for server and client? Perhaps there's an easier way that works today anyway - like having a function that you replace with a "browser" version that no-ops when the server implementation exists?

kimamula commented 8 years ago

Thank you for the thoughtful reply. I have been rethinking about my requirement and how it can be fulfilled with a simple solution.

I realized that at least I can mimic my proposed solution by wrapping free-style as follows.

import { create } from 'free-style';

class ServerSideFreeStyleWrapper {
  private keyProvider = create();
  private style = create();
  map: {[key: string]: string} = {};

  constructor(private processors: ((userStyles: UserStyles) => UserStyles)[]) {}

  registerStyle(userStyles: UserStyles): string {
    const key = this.keyProvider.registerStyle(userStyles);
    const value = this.style.registerStyle(this.processors.reduce(
      (userStyles, processor) => processor(userStyles), userStyles
    ));
    this.map[key] = value;
    return value;
  }

  getStyles(): string {
    return this.style.getStyles();
  }
}

class ClientSideFreeStyleWrapper {
  private keyProvider = create();
  // Constructor accepts the map generated on the server side.
  constructor(private map: {[key: string]: string}) {}

  registerStyle(userStyles: UserStyles): string {
    return this.map[this.keyProvider.registerStyle(userStyles)];
  }

  getStyles(): string {
    // Dynamic CSS generation on the client side is inhibited in this use case.
    // App keeps using CSS generated on the server side.
    throw new Error('getStyles cannot be used on the client side');
  }
}

I confirmed this works fine and therefore there's no need to modify free-style itself now.

I would appreciate if you have any advice.

blakeembrey commented 8 years ago

Can you elaborate a little more on the goal of the two separate keys? Once I understand that, I can try figure out how to make this usable for you without any major trade-offs. Also, the pre-processors are all be using the object format right? I think that would mean a refactor on what content is used for hashing. Aside from that, you could have multiple instances and copy styles from one to the other - at what point does the processors behaviour occur? I'm guessing that if I mount a style on one instance and then copy it to another - it should run the processors on the new instance? Or should I stay as it was intended?

I'd have to implement it, but here's some context. Looking at https://github.com/blakeembrey/free-style/blob/master/src/free-style.ts#L181-L225, it's the main loop where all styles are currently generated and the hash computed. I think it's important that internally, the hashes remain based the styles themselves (that's how de-duping occurs - every selector, style, rule, etc is also hashed). However, in this case, the feature only needs to augment the user-hash - which is a hash of the "combined" styles string. Given this, it means that the combined styles string could remain the exact user input while internally we apply processors.

What does that look like? I think it only means modifying these lines to de-couple the pid from the stringified properties - https://github.com/blakeembrey/free-style/blob/master/src/free-style.ts#L186-L187. I may also have to move away from the internal tuple representation, unless that's ideal for preprocessor support?

Remaining questions:

  1. What should the pre-processor object look like? This would heavily impact what the implementation looks like since free-style splits nested styles apart from properties and the above implementation relies on that behaviour.
  2. What's the behaviour for different instances? The proposal above relies on the fact the internal representations are consistently hashed - which I think is important to keep. Internally, there's never been hash re-computation but changing this might require that to occur.
blakeembrey commented 8 years ago

Looking at https://github.com/postcss/postcss-js, it might be best to keep the current object model of selectors, repeat properties and values in place. What do you think? If that happens, the simple solution here is that I just do const className = hash(JSON.stringify(userStyle)) and then pass the entire object to the processors and let the rest of the project do it's thing. Assuming object -> object representation, it means there's very minimal changes to free-style to make this happen and the only real change is de-coupling the output hash from the output object (currently only the input object can equal the output object, so it wasn't an issue before).

kimamula commented 8 years ago

Can you elaborate a little more on the goal of the two separate keys?

Do you mean keys created by keyProvider which appeared in my sample code? This is used to enable obtaining "real" class names (i.e. hashes generated from the processed styles) on the client side. As I want CSS-processing to take place only on the server side, an associative array is created on the server side in which the "real" class names are keyed with some unique hash that can be easily generated without depending on any huge library (the keys are not necessarily generated by free-style), which is then passed to the client side.

the simple solution here is that I just do const className = hash(JSON.stringify(userStyle)) and then pass the entire object to the processors and let the rest of the project do it's thing.

How is the returned className used? Does it become a "real" class name which appears in the CSS output of Style.getStyles()? Or is it used like a key in my sample code? Also, I wonder if the hash function can detect hash collision.

you could have multiple instances and copy styles from one to the other - at what point does the processors behaviour occur?

I have been totally missing this point. How about passing processors as arguments of Style.getStyles()? Upon calling registerStyle(userStyle), free-style just returns the hash and internally store the passed style (probably in an associative array which is keyed with the hash). Appling processors to styles and generating CSS output is not occurred until calling getStyles(). According to this design, the behavior would be obvious and predictable, wouldn't it?

blakeembrey commented 8 years ago
  1. The returned className is used just the same as before
  2. Yes, it's a real class name - that's how it works today (the class name is a hash of everything while every rule/selector/style has it's own hash to de-dupe - it's how free-style can de-dupe inside of media queries even though they might be from two different style instances)
  3. I don't think so. I think I understand the key implementation now - it's a map to styles that exist with additional processing, right? So, on the client, you'd have strings that are consistent with the server mapping, which maps to styles after the post-processing applied. I'll need to think about how this works better, but ideally it sounds like a) all styles come from the server with post-processing applied and b) the client doesn't need to store any styles, only keys for the correct styles?
  4. hash doesn't detect collisions, but the Cache class itself does - see https://github.com/blakeembrey/free-style/blob/master/src/free-style.ts#L320-L323. It does this by checking the key itself and then comparing the contents
  5. getStyles could be passed as arguments, but you end up processing the same things every time you render which isn't optimal
  6. Yes, that's possible - I just don't understand the use-case enough. I get the idea, but there's more optimal solutions that exist depending on what you're trying to do, because that's how it works today except I've minimised the memory overhead by making the CSS itself part of the cache (that way, there's only one representation in memory needed, not an object and a hash and CSS text output on each getStyles call - which can happen often if you navigate a lot or use dynamic styles)
blakeembrey commented 8 years ago

Coming back to the goal, it seems to be this:

  1. Generate styles on the server-side with additional processing
  2. Have some way to deterministically access the server-side style from the client-side
  3. Use the server-side class name on the client-side to have all the post-processing applied already

Given this, perhaps there's a better way to ship something that isn't object and hashing on the client-side? What if you used objects and the server-side process does additional processing while the client-side literally returns strings? No more objects needed for finding the hashes, and you don't even need to map anything anymore. Does that make sense or am I missing something?

kimamula commented 8 years ago

I agree with you on all the 3 points you mentiond above. However, I can't understand what you mean in thease sentenses.

What if you used objects and the server-side process does additional processing while the client-side literally returns strings? No more objects needed for finding the hashes, and you don't even need to map anything anymore.

Could you give me a little more additional explanation?

As for my use-case, I don't need to call getStyles repeatedly. As I don't use dynamic styles, it is possible that I execute all registerStyle calls upon server starts and then call getStyles only once, whose result is cached and used in every HTML response. But I understand free-style should perform effectively even if one calls getStyles many times, so probably the solution I suggested is not good.

blakeembrey commented 8 years ago

I might be mis-understanding the client-side implementation, but it seemed like you have objects on the client-side that you use to get the hash which is then mapped to the server-side hash. I was trying to suggest that perhaps you could remove the need from having objects on the client-side at all, but perhaps I missed something. How does the client-side access and use the server-hashes today?

kimamula commented 8 years ago

Yes, I have an object on the client side. My current implementation is something like the following (ServerSideFreeStyleWrapper and ClientSideFreeStyleWrapper are described here).

let freeStyleWrapper: FreeStyleWrapper;
export function initialize(_freeStyleWrapper: FreeStyleWrapper) {
  freeStyleWrapper = _freeStyleWrapper;
}
export function get() {
  return freeStyleWrapper;
}
import { get } from './Style';

const className = get().registerStyle({/* some style definition */});
export class Component extends React.Component<void, void> {
  render() {
    return <div className={className}>Hello world!</div>;
  }
}
import { initialize } from './Style';

const prefixer = postcssJs.sync([ autoprefixer ]);
const wrapper = new ServerSideFreeStyleWrapper([ prefixer ]);
initialize(wrapper);

import { Component } from './Component';

app.get('/', (req, res) => res.send(`<!DOCTYPE html>${renderToStaticMarkup(
  <html>
    <head>
      <style type='text/css'>{wrapper.getStyles()}</style>
    </head>
    <body>
      <div id='app'
        data-map={JSON.stringify(wrapper.map)}
        dangerouslySetInnerHTML={{__html: renderToString(<Component />)}}
      ></div>
      <script type='text/javascript' src='bundle.js'></script>
    </body>
  </html>
)}`)
import { initialize } from './Style';

const root = document.getElementById('app');
const map =JSON.parse(root.dataset['map']); // this is the client-side object
initialize(new ClientSideFreeStyleWrapper(map));

import { Component } from './Component';

ReactDOM.render(<Component />, root);
blakeembrey commented 8 years ago

Thanks for all the sample code, it's similar to what I expected but good to know I'm not about to overload you with irrelevant implementation details 😄 Do you use Webpack for the front-end by the way?

kimamula commented 8 years ago

Yes, I use Webpack.

If I could could remove the need from having objects on the client-side, that would be wonderful!

blakeembrey commented 8 years ago

Awesome! The reason I ask is this. What if we went in and built a webpack-free-style or something similar? If you use that directly, we can transform all calls to .create() and .registerStyle, etc. into strings and collect the styles from Webpack to write directly to a CSS file (or somewhere else). I believe the types of objects that can be processed might be limiting though, so would need to investigate that. However, the advantage is the client only has strings while the server can do everything it needs to.

Because of the limited types we can transform statically, we may need to add some restrictions to how free-style is allowed to be used. For example, if we put styles into JSON files that would make transforms 100x easier. I'll think on it and update this issue for that, but it seems the current solution you landed on works - we're just trying to optimise now.

kimamula commented 8 years ago

webpack-free-style sounds good, but I wonder what advantages it has over CSS Modules. One of the main advantages of free-style has over CSS Modules is, I suppose, that you can write everything in JavaScript, which enables constants sharing between CSS and JavaScript and provides all the extensibility of JavaScript. I hope this advantage would remain as much as possible in webpack-free-style.

blakeembrey commented 8 years ago

Great points. I'll think on it and open a new issue to solve it and keep this one open for consideration. I wouldn't want to break the features that make free-style what it is and I'd like to enable something more powerful, and maybe building something like CSS modules will enable a different set of people (the ones that use CSS modules today, I suppose). Either way, I'm not sure I'll get the time personally, just wanted to throw out the idea.

The trickiest thing here seems to be that we need a way to evaluate all the styles dynamically and then be able to replace those styles in the build. However, it's not ideal if we'd run JavaScript to collect the styles and then pass it into a build tool. Anyone have any ideas?

In terms of the original idea that the same hashes could be generated with different post-processors, it should be very simple to enable (maybe even in a backward compatible way, but most likely I'll need to break the current hash outputs). I can't think of any solid reasons not to enable it, so I think you can see it soon 😄

kimamula commented 8 years ago

I can't think of any solid reasons not to enable it, so I think you can see it soon 😄

That's great. Thanks!

I hope I can help you solving the new issue, though currently I have no idea how to solve it.