`class` property was broken, like pug`div( class="container" )`

shirohana commented 5 years ago

As title, seems the class property alias was broken since ^7.0.0 but works in ^6.0.0.

It seems like the removal of alias class -> className in #62?

`src/visitors/Tag.js`

+// TODO: Need to drop all aliases for attributes
 switch (name) {
   case 'for':
     name = 'htmlFor';
     break;
   case 'maxlength':
     name = 'maxLength';
     break;
-  case 'class':
-    name = 'className';
-    break;
 }

Here's the sample:

It works when using `className` (REPL)

// import $ from './style.css'
const $ = { /* as CSSModule */ }

export function Link () {
  return pug`
    div( className=$.Container )
      a( className=[$.Link, $.LinkActive] )
  `
}

And it broken when `class` but it should be work (REPL)

// import $ from './style.css'
const $ = { /* as CSSModule */ }

export function Link () {
  return pug`
    div( class=$.Container )
      a( class=[$.Link, $.LinkActive] )
  `
}

Full log from my console

Module build failed (from ./node_modules/babel-loader/lib/index.js):
Error: /Users/shirohana/Repositories/react-spa-template/src/components/dev-tools/sitemap.js:41
    39|   return pug`
    40|     div( class=$.Container )
  > 41|       a( class=[$.Link, $.LinkActive] )
    42|   `
    43| }
    44|

We can't use expressions in shorthands, use "className" instead of "class"
    at makeError (/Users/shirohana/Repositories/react-spa-template/node_modules/pug-error/index.js:32:13)
    at Context.error (/Users/shirohana/Repositories/react-spa-template/node_modules/babel-plugin-transform-react-pug/dist/context.js:63:34)
    at node.attrs.map (/Users/shirohana/Repositories/react-spa-template/node_modules/babel-plugin-transform-react-pug/dist/visitors/Tag.js:81:23)
    at Array.map (<anonymous>)
    at getAttributes (/Users/shirohana/Repositories/react-spa-template/node_modules/babel-plugin-transform-react-pug/dist/visitors/Tag.js:45:28)
    at getAttributesAndChildren (/Users/shirohana/Repositories/react-spa-template/node_modules/babel-plugin-transform-react-pug/dist/visitors/Tag.js:125:17)
    at Object.jsx (/Users/shirohana/Repositories/react-spa-template/node_modules/babel-plugin-transform-react-pug/dist/visitors/Tag.js:166:35)
    at visitJsx (/Users/shirohana/Repositories/react-spa-template/node_modules/babel-plugin-transform-react-pug/dist/visitors.js:82:20)
    at nodes.forEach (/Users/shirohana/Repositories/react-spa-template/node_modules/babel-plugin-transform-react-pug/dist/visitors.js:58:19)
    at Array.forEach (<anonymous>)

ezhlobo commented 5 years ago

@shirohana my intention was to avoid own aliases and just pass everything what we have to jsx. With class it's tricky since jsx didn't allow us to use it and now it throws warning, but it works well there. Maybe we should consider adding an alias for class attribute.

Do you prefer using class just because it's shorter or you have other reasons?

shirohana commented 5 years ago

It it surprised me that the class has been removed without documented (´・ω・｀)

The class should keep be available just because we love Pug more than JSX, and class is a Syntax of Pug, not an alias.

Just because we can use Pug as our preprocessor of JSX, we can avoid some bad design of JSX, like ugly optional rendering condition, ugly array iteration, meaningless closing-tag, and so many reasons that drive us to find a better workaround, that is why this plugin be existed.

shirohana commented 5 years ago

We're looking for a solution, not another JSX-like alternatives. (´・ω・｀ʃ♡ƪ)

shirohana commented 5 years ago

Is it any plan for this (´・ω・｀)？

billypon commented 5 years ago

@ezhlobo Could you give us an option that we can map class to className, default is not map? I prefer using class. Passthrough everything is right, because class and id are just html attributes. But map class to className also is right, it can be a syntax of pug, and it's shorter. We are writing pug, not jsx, when we use this plugin.

ezhlobo commented 4 years ago

@shirohana @billypon hey ✋, I feel sad that I made you waiting for too long.

I thought a bit about that and now I tend to agree that we need to have a map between class and className. I'll check it out tomorrow or after that.

Also, the message like We can't use expressions in shorthands, use "className" instead of "class" points to an issue in the logic, nothing about the avoidance of aliases.

vanBrakel commented 4 years ago

@ezhlobo In case you want to add the class -> className mapping back, then wouldn't it also make sense to add mapping for all other CSS attributes?

shirohana commented 4 years ago

@JeanPaulvB These all other CSS attributes were created for JSX that makes almost DOM Element Attributes to be a valid JS Identifier (like variable names), but Pug is just a HTML-preprocessor, no need to be compatible with JavaScript, as I think (´・ω・｀)

vanBrakel commented 4 years ago

@shirohana But the PUG attributes do need to be compatible with React right? Hence a pure Pug to JSX mapping (like class to className) could be very nice.

shirohana commented 4 years ago

@JeanPaulvB Yeah that is what I expect this plugin does \:D

✅I'd like that do:

.container
  label( for="email" )
  input#email( type="text" )

❌but not:

.container
  label( html-for="email" )
  input#email( type="text" )

❌and never:

.container
  label( htmlFor="email" )
  input#email( type="text" )

this plugin done that pretty well in this example, and I hope it can also applies on class -> className ❤️

danielnarey commented 4 years ago

Any plans to revert to allowing “class” (as in Pug language spec) in place of “className” (as in JS/JSX)?

shirohana commented 4 years ago

Hi, I'm back here for React and frontend ヾ(*´ω｀*)ノ

Currently I have a plan to make a new option to allow using:

pug`div.box`,
pug`div( class="box" )`,
pug`div( className="box" )` and
pug`div( class=styles.box )` (for CSS Module)

in same time, just like alias that works well before.

with options like:

`babel.config.js`

{
  "plugins": [
    ["transform-react-pug", {
      "attributeAlias": {
        "KEY": "VALUE",
      }
    }]
  ]
}

Which:

KEY: Attribute name of HTML Element which is invalid in JSX and listed in React Doc.

For example:
- "class" replaced by className
- "for" replaced by htmlFor
VALUE: The replacement string that should be valid in JSX.

This way provides the most flexibility of use cases, you can setup for JSX-like syntax, Pug-like syntax, and even more power that Pug doesn't have:

JSX-like

"attributeAlias": {
  /* No need to configure, unlisted attributes will be fallback as it is */
}

pug`div( className="box" )` // valid
pug`div( class="box" )` // invalid

pug`label( htmlFor="username" )` // valid
pug`label( for="username" )` // invalid

Pug-like

"attributeAlias": {
  "class": "className",
  "for": "htmlFor"
}

pug`div( className="box" )` // valid
pug`div( class="box" )` // valid, will be parsed to JSX `<div className="box"></div>`

pug`label( htmlFor="username" )` // valid
pug`label( for="username" )` // valid, will be parsed to JSX `<label htmlFor="username"></label>`

More power

"attributeAlias": {
  "@click": "onClick",
  "@change": "onChange",
  "@html": "dangerouslySetInnerHTML"
}

pug`button( @click=handleClick )` // valid, will be parsed to JSX `<button onClick=handleClick></button>`
pug`svg( @html=canvas )` // valid, will be parsed to JSX `<svg dangerouslySetInnerHTML=canvas></svg>`

Default alias will be like that for compatible with nature syntax of Pug:

"attributeAlias": {
  "class": "className",
  "for": "htmlFor"
}

I'll try to implement in few days, hope that can match everyone's preference easily.

Welcome for any advice ٩(｡・ω・｡)

p-98 commented 2 years ago

@shirohana @ezhlobo Are there any updates or reasons why the PR is not merged?

shirohana commented 2 years ago

@p-98 I have no idea, guessing this project was been deprecated with no announcements. and I'm gone for poor type checking from pug months ago 😉

pugjs / babel-plugin-transform-react-pug