lukejacksonn / oceanwind

Compiles tailwind shorthand into css at runtime. Succeeded by Twind.
https://twind.dev
264 stars 12 forks source link

memoize process calls? #4

Open giuseppeg opened 4 years ago

giuseppeg commented 4 years ago

Hey awesome project! I think that process invocations could be memoized in https://github.com/lukejacksonn/oceanwind/blob/master/index.js (a simple strict equality check would do it) - this way you don't have to translate all the time.

Although the switch is super cheap so probably this optimization is unnecessary 🙈

lukejacksonn commented 4 years ago

Hey 👋 you are right! This certainly is an optimization I've thought about.. although, what do you think about memoizing the translate calls instead? I figure you are likely to be asked for a single directive multiple times, than a list of directives..

What do you think?

giuseppeg commented 4 years ago

Ah yeah that'd work too!

lukejacksonn commented 4 years ago

It will surely have an impact on the memory footprint of the average use case. This is pretty much the only reason I am wary about doing it.. my gut feeling is that memoization of directives is probably going to work out better overall, but it would be nice to know for sure!

Getting some benchmarks setup would be useful here.

giuseppeg commented 4 years ago

Is process(theme) idempotent? or does its return value change based on some context value (eg. for dark mode)?

If it is idempotent you could just do the following:

// If passed a theme then return process primed with merged themes
export const themed = (input) => {
  const theme = merge(defaultTheme, input);
  const _process = process(theme)
  return (input) => css(process(input));
};
const defaultProcess = process(defaultTheme)
// If passed a tagged template then process with the default theme
export default (input) => css(defaultProcess(input));
lukejacksonn commented 4 years ago

It is idempotent. Once you have primed the process function with a theme that's it. You can just pass it directives from then on. I guess you are trying to avoid curried function call here? I wonder if that makes any big difference perf wise 🧐