[core] Replace classnames utility with clsx dependency

[ZeeshanTamboli]

Part of #6 : To align convention with the main core repository. Also faster execution - clsx performance benchmarks
Part of #18 : Bundle size reduction. Not sure if classnames custom utility is smaller or clsx dependency.

Looked worthwhile to do this now and as long as the codebase is small it would be less cumbersome to replace now than doing it later when codebase grows.

Note: We already also use the babel-plugin-optimize-clsx
JedWatson/classnames#187 (comment)

[dtassone]

It would have been nice to actually keep classnames as a wrapper of clsx, and isolate the dependency in utils/classnames, so we would have had an isolated pluggable dependency, so it's easy to switch the underlying implementation.

[oliviertassinari]

so it's easy to switch the underlying implementation.

Why is making it easy to switch something to optimize for? I mean it seems that it was simple enough in this very occurrence. Could it make it more confusing for developers when contributing between different components? TreeView, Scheduler, etc.

[dtassone]

so it's easy to switch the underlying implementation.

Why is making it easy to switch something to optimize for? I mean it seems that it was simple enough in this very occurrence. Could it make it more confusing for developers when contributing between different components? TreeView, Scheduler, etc.

Bundle size reduction. Not sure if classnames custom utility is smaller or clsx dependency. => Just to be able to easily switch component by adding an abstraction that could point here or there.

In the past, we have changed the debounce function several times.

[ZeeshanTamboli]

Maybe one advantage of having a dependency is that the performance improvements are handled by the contributions to clsx. We need not spend time on it. Like this PR increases performance as per the metrics posted. (Although it's been long and it's still not yet accepted/merged).

I created this PR as per requirements in #6 and #18. What about #6 : aligning with the main repo? Wouldn't it be a concern even if the underlying implementation here is different? Keeping custom implementation so that DataGrid is as much optimised as possible? Or is it like pasting the same clsx code and keeping it open for edit/customisation?

[oliviertassinari]

Not sure if classnames custom utility is smaller or clsx dependency.

@dtassone the data grid imports clsx by importing the core components. So classnames + clsx (HEAD) will always be larger than clsx only (PR), hence why the bundle size win of this PR. Also, a good reminder to work on #37's item "bundle size diff", so we can see the actual win right from the PR review with an automatic report.

to be able to easily switch component by adding an abstraction that could point here or there.

@ZeeshanTamboli Was doing the change in this PR, easy enough?

[ZeeshanTamboli]

@ZeeshanTamboli Was doing the change in this PR, easy enough?

@oliviertassinari Yes.

[dtassone]

Not sure if classnames custom utility is smaller or clsx dependency.

@dtassone the data grid imports clsx by importing the core components. So classnames + clsx (HEAD) will always be larger than clsx only (PR), hence why the bundle size win of this PR. Also, a good reminder to work on #37's item "bundle size diff", so we can see the actual win right from the PR review with an automatic report.

to be able to easily switch component by adding an abstraction that could point here or there.

@ZeeshanTamboli Was doing the change in this PR, easy enough?

Classname.ts

import clsx from 'classnames'
export classnames= clsx

Done, no need to change all the files.
And if you want to unplug clsx, it is only in one file.
let say you want to replace it with material-ui/utils, you just have to change 1 file.
That's what I meant.