Add decss-style API #23

taion · 2018-06-07T04:20:09Z

taion · 2018-06-07T04:20:32Z

example/src/Button.js

@@ -20,13 +19,13 @@ const ButtonBase = styled('button')`
  white-space: nowrap;
  user-select: none;

-  ${p => p.theme === 'primary' && css`
+  &.theme.primary {


so, as long as you have nesting enabled, this decss-like API works

ok, so this also would let you do primary="theme", but... come on

taion · 2018-06-07T04:20:50Z

src/plugin.js

@@ -99,37 +81,6 @@ export default function plugin() {
    return style;
  }

-  function extractNestedClasses(path, className, cssState, hoistImport) {


i figure this isn't super necessary if we have the nesting-based API?

taion · 2018-06-07T04:21:26Z

src/runtime/styled.js

+    delete childProps.innerRef;
+    childProps.ref = props.innerRef;
+
+    if (hasModifiers) {


probably better to iterate through props rather than iterating through defined styles?

taion · 2018-06-07T04:24:25Z

src/runtime/styled.js

+        ) {
+          if (propValue === true) {
+            modifierClassNames.push(styles[propName]);
+          } else if (propValue !== false && has.call(styles, propValue)) {


maybe we should camel-case propValue strings (and perhaps propName... though anybody using non-camel-case prop names but enabling camel-casing in the css modules transform deserves what they get)

taion · 2018-06-07T04:34:18Z

src/runtime/styled.js


 export default styled;
-
-export const css = () => {


bleh i didn't realize the docs suggested using this in all cases. should i add it back?

taion · 2018-06-07T04:36:47Z

README.md

+      border: 1px solid blue;
+    }
+
+    &.color.green {


we could do the decss thing of .color-green as well... that would definitely require pulling in lodash/camelCase, though. also it wouldn't let us delete the prop if the value doesn't match. it would avoid the weird precedence things here, though, and it'd prevent green="color" from working

I am not sure i understand the extent to which this all works...what is cruz of the logic, you check all classes and see if they map to a name or a bool prop?

It's not super obvious to me that .color here is functioning as a prop name, while green is the value, even tho both and the inverse work as well

+1, &.color=green?

it'd have to be .color-green, and because of camelcasing things it'd get munged into .colorGreen, and we'd support both color="green" and colorGreen={true} as props, but that's not too important

right now the logic is going through all the props and seeing which ones match classes

I like that better, but it's probably not possible without adding a css parser. My only concern there is we have no idea waht flavor of css the user is writing so it's tough to try and parse it (e.g. LESS, SCSS, custom postcss) I don't know that we'd get a reliable AST without user imput, but you'd be the expert there :P

yeah so i think i'll just do the dashed class names and pull in lodash/camelCase? so you'll do .theme-green, but we'll look for .themeGreen and .theme-green when you do theme="green"

I like that better, the cost of the lodash function will be small enough

jquense · 2018-06-07T12:34:00Z

Idk this API seems strictly worse no? It's the same for the simple case but doesn't allow any complex prop to class mappings or dynamic ones

ai · 2018-06-07T12:36:19Z

I like it 😍

It is more readable than styled-components

jquense · 2018-06-07T12:37:27Z

I don't see the win here, it's a lot more magic seeming...and less control

ai · 2018-06-07T12:37:34Z

I like this API even if it is limited because you don’t need dynamic properties in most of the cases. Most cases are simple. So we should have the simple solution for them.

ai · 2018-06-07T12:43:56Z

@jquense what do you think of keeping both API? Simple &.prop for simple cases and template string ${ prop && … } for the complex with dynamic values

taion · 2018-06-07T15:21:26Z

well, prettier doesn't mess this format up :p

in practice every case we have is either props.foo === 'bar' or props.foo, no?

we probably can't go much beyond this without something like a CSS parser, but the simplicity is worthwhile.

jquense · 2018-06-07T15:23:36Z

example/.eslintrc

@@ -2,5 +2,8 @@
  "extends": "4catalyzer-react",
  "env": {
    "browser": true
+  },
+  globals: {
+    "css": false


no globals!

so does that mean all of our code is using this the wrong way by using this as a global? ;)

its using it in a "deprecated" way, I left the auto-assume global for the bare css to not break our code, but most every other lib requires an import, which i think makes sense

taion · 2018-06-07T21:53:05Z

@jquense should we keep the old API or not? it makes it easier to tell whether there are modifiers if we only have the "normal" class name, but i guess we could look for the class names that start with "variant" or whatever

jquense · 2018-06-07T22:37:58Z

We can remove it for now. Always easily add it back if needed

taion · 2018-06-09T04:57:26Z

updated... this is the same API as decss now, minus the part where it breaks if you only emit camelcased class names

taion · 2018-06-13T14:46:18Z

Thoughts?

ai · 2018-06-13T14:51:43Z

I like new prop-value syntax

jquense · 2018-06-13T14:56:23Z

test/__snapshots__/loader.test.js.snap


+  &.primary {


this essentially requires that you have a nesting feature compile set up, which means no usage with plain css..

jquense · 2018-06-13T14:59:02Z

this api is nice, but it does require the user now have some specific preprocessor set up in addition to css-modules, at the very least it requires postcss-loader

jquense

i'm ok with giving this a shot, we should update the docs to say that nesting is required no?

jquense · 2018-06-21T13:18:53Z

maybe we should support both tho?

taion · 2018-06-21T15:50:17Z

make up your mind #23 (comment) 🤣

jquense · 2018-06-23T18:14:45Z

lets give it a shot 👍

jquense · 2018-06-23T18:14:59Z

we can add back the old api as well if the double use-case arrises

kossnocorp · 2018-06-27T16:25:42Z

Awesome! 👏

taion added 3 commits June 7, 2018 00:18

Add decss-style API

78335e8

update tests

a22bab9

Add docs

bac1471

taion commented Jun 7, 2018

View reviewed changes

wording tweak

48860ff

taion commented Jun 7, 2018

View reviewed changes

jquense reviewed Jun 7, 2018

View reviewed changes

taion added 2 commits June 9, 2018 00:53

More like decss

1bbe360

tweak

86b56d0

jquense reviewed Jun 13, 2018

View reviewed changes

add back export

4ff27d2

jquense approved these changes Jun 21, 2018

View reviewed changes

jquense merged commit 770ba48 into master Jun 23, 2018

taion deleted the decss branch June 23, 2018 21:06

taion mentioned this pull request Jul 2, 2018

decss #20

Closed

Add decss-style API #23

Add decss-style API #23

Conversation

taion commented Jun 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ai Jun 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jquense commented Jun 7, 2018

ai commented Jun 7, 2018

jquense commented Jun 7, 2018

ai commented Jun 7, 2018

ai commented Jun 7, 2018

taion commented Jun 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

taion commented Jun 7, 2018

jquense commented Jun 7, 2018

taion commented Jun 9, 2018

taion commented Jun 13, 2018

ai commented Jun 13, 2018

jquense Jun 13, 2018 • edited Loading

Choose a reason for hiding this comment

jquense commented Jun 13, 2018

jquense left a comment

Choose a reason for hiding this comment

jquense commented Jun 21, 2018

taion commented Jun 21, 2018

jquense commented Jun 23, 2018

jquense commented Jun 23, 2018

kossnocorp commented Jun 27, 2018

ai Jun 7, 2018 •

edited

Loading

jquense Jun 13, 2018 •

edited

Loading