search

aviflombaum / shadcn-rails

https://shadcn.rails-components.com
MIT License
570 stars 35 forks source link

Possible performance issue with how TailwindMerge is used #26

Closed j-manu closed 1 year ago

j-manu commented 1 year ago

I am also building a Rails UI Kit, also a port of shadcn. It started quite some time back and your library is providing motivation to complete it :) I have a very different approach from yours but I also use Tailwind merge.

When I used it like you did 

def tw(classes)
    TailwindMerge::Merger.new.merge(classes)
 end

it had a negative impact on production performance. I had to memoize it to get good performance. Also, the performance optimizations of the gem - https://github.com/gjtorikian/tailwind_merge#performance - kicks in only if you the same instance AFAIK.

It could be that, this is specific to my implementation. I haven't tested shadcdn-rails.

aviflombaum commented 1 year ago

Wow, I'd have to profile to see if I'm getting a performance hit for using it. I don't know where I'd memoize it as this isn't being used within an object that instantiates or is a class. Reading a bit more about the gem, I can see why it might be a performance issue to use this on every page load. How did you profile to see the implications of this in production? Happy to combine efforts.

j-manu commented 1 year ago

  1. I didn't profile systematically. My test pages were extremely slow in production and one of the first things I tried was turning off TailwindMerge and that fixed it. Thankfully, memoizing it made it fast again.

  2. I am using a module to namespace and the use this method definition in the module and call it directly. My only worry is thread safety. I will need to investigate that later.

def self.tailwind_merger
    @tw_merger ||= TailwindMerge::Merger.new
  end
  1. I would have loved to combine efforts, but firstly my UI kit will be paid (Unfortunately can't afford open source work financially). I had chatted with shadcn before I started and they were supportive. Secondly, apart from the UI layer, we are taking very different approaches. I am building it using Phlex for one. I am happy that your project exists and your progress has been very inspiring and motivating.
aviflombaum commented 1 year ago

I'm going to close this unless someone else shows me performance comparisons.