bluewings/pug-as-jsx-loader

CSS Modules aren't "puggy"

Closed this issue · 2 comments

d3dc commented

I think you've created an awesome loader. At the very least, it gives me an excuse to enforce the separation of presentation and container components.

I really like that you added a way to do imports. It actually works for things like images really well.

For its intended use case, however, the usability could be improved.

  1. In a simple app, maybe I don't want css modules:
// @import ./button-theme.css

button.btn.btnPrimary ...
  1. It also doesn't feel right when classes usually come before attributes and I have to write:
div(className='{classnames("btn", styles.btnPrimary)}')

Perhaps it could be another macro:

// @styles ./button-theme.css

button.btn.btn-primary ...

This would need to template ${ styles['btn'] || 'btn' } ${ styles['btn-primary'] || 'btn-primary' } under the hood at runtime, right?

@d3dc

I agree with the necessity of your suggestion.

In most cases, however, local and global classes are used together within a template, in which case the above example is accompanied by ambiguity.
(You can not tell if the class is local or global without looking at the 'css' file.)

It is difficult to provide the above method in common, but it seems to be applicable as follows.

webpack.config.js (The transformClassName option allows the user to define the class naming convention.)

...
{
  test: /\.pug$/,
  use: [
    require.resolve('babel-loader'),
    {
      loader: require.resolve('pug-as-jsx-loader'),
      options: {
        transformClassName: (classname, styles) => {
          return styles[classname] || classname;
        },
      },
    },
  ],
},
...

sample.pug

// @import .css => styles
div 
  .foo
  .foo.bar
  div(className='{bar}')
  div(className='{"foo " + bar}')
  div(className='{cx("foo", bar)}')

transpiled

import styles from './sample.scss';

const __macro_classname = (classnames) => {
  if (typeof classnames !== 'string') {
    return '';
  }
  return classnames.split(/\s+/)
    .map(classname => __transformClassName(classname, styles))
    .join(' ');
};

export default function (params = {}) {
  const { bar, cx } = params;
  return (
    <div>
      <div className={__macro_classname("foo")} />
      <div className={__macro_classname("foo bar")} />
      <div className={__macro_classname(bar)} />
      <div className={__macro_classname('foo ' + bar)} />
      <div className={__macro_classname(cx('foo', bar))} />
    </div>
  );
};

❗The above features have not yet been implemented. If you give me your feedback, I will consider it for future updates.

d3dc commented

Sorry I never replied to this, I've been off exploring separation of presentational concerns in my own way.

If I follow your implementation, it would be the flexible way to implement what I said. I am just not sure what implications it might have.

One solution I found was just a separate prop styleName to avoid the ambiguity. But I guess that would mean extending the grammar for a shorthand.

The solution I'm currently playing with involves my own library. I've been thinking about packaging CSS modules as react components. I have mixed feelings about this when I'm writing javascript. But IMO conflating styles might make sense inside a .pug

// Comp is global
// @import menu.css => Menu

Comp(w='auto'  shadow='lg'): Menu(bg='dark')
  Menu.Item(active)| dogs
  Menu.Item| cats

renders as

<div className="w-auto shadow-lg Menu Menu__bg-dark">
  <div className="Menu--item Menu--item__active">dogs</div>
  <div className="Menu--item">cats</div>
</div>