bradlc / babel-plugin-tailwind-components

Use Tailwind with any CSS-in-JS library
MIT License
332 stars 25 forks source link

Update to beta.8 #24

Closed DevanB closed 5 years ago

DevanB commented 5 years ago

This PR updates the tailwind dependency to 1.0.0-beta.8.

bradlc commented 5 years ago

Hey @DevanB, thanks for this.

There’s a bit more to it than that though. We need to look at the updates since beta 4 and implement any changes. For example beta 6 added order utilities. This will need adding to dynamicStyles.js

If you’re interested in helping out I can put together a list of things that need doing? If not, no worries, I can tackle it this weekend probably

DevanB commented 5 years ago

@bradlc, I’d be thrilled to!

I did take a gander about the codebase before sending this in and thought “he will let me know if it’s incorrect, otherwise, I can correct it”.

So yes, I’d love to help update and contribute since I’m actively using the macro :)

DevanB commented 5 years ago

Looking now:

beta.5:

beta.6:

beta.7:

beta.8:

So, I think @bradlc that maybe order and negative margin, inset, and z-indexes are the only changes necessary to add to this PR?

bradlc commented 5 years ago

Thanks for putting together a list @DevanB !

I think you’re right: we need to add the order module and add support for keys that start with -.

Here is where we determine the prefix and key for a class name:

https://github.com/bradlc/babel-plugin-tailwind-components/blob/f4a796b42bcf9b9bfd44cd905739982a2e244254/src/transformString.js#L63-L76

Currently if you have the class name -m-1 then prefix = '-m', key = '1'. But now, we want it to be prefix = 'm', key = '-1'

DevanB commented 5 years ago

If you don’t mind @bradlc, I’ll update my PR to take a stab at it all :smile:

bradlc commented 5 years ago

Go for it! Let me know if you have any questions

DevanB commented 5 years ago

@bradlc I believe I've correctly added order via my PR.

Working on the negatives now, but I noticed a utility for negativeMargin here:

https://github.com/bradlc/babel-plugin-tailwind-components/blob/f4a796b42bcf9b9bfd44cd905739982a2e244254/src/transformString.js#L98-L121

Since Tailwind removed this, I assume the config might go from:

https://github.com/bradlc/babel-plugin-tailwind-components/blob/f4a796b42bcf9b9bfd44cd905739982a2e244254/src/dynamicStyles.js#L86-L100

to this (not sure if pre is necessary anymore):

  '-mt': { prop: 'marginTop', config: 'margin', pre: '"-"+' },
  '-mr': { prop: 'marginRight', config: 'margin', pre: '"-"+' },
  '-mb': { prop: 'marginBottom', config: 'margin', pre: '"-"+' },
  '-ml': { prop: 'marginLeft', config: 'margin', pre: '"-"+' },
  '-mx': {
    prop: ['marginLeft', 'marginRight'],
    config: 'margin',
    pre: '"-"+'
  },
  '-my': {
    prop: ['marginTop', 'marginBottom'],
    config: 'margin',
    pre: '"-"+'
  },
  '-m': { prop: 'margin', config: 'margin', pre: '"-"+' },

Currently if you have the class name -m-1 then prefix = '-m', key = '1'. But now, we want it to be prefix = 'm', key = '-1'

But then I saw your comment, and now I'm a bit lost 😄

DevanB commented 5 years ago

Additionally, I've tried building and linking, however there seems to be a flow types issue (I believe an issue in microbundle). Curious if you are able to build the project.

bradlc commented 5 years ago

Hey @DevanB. It won’t build because you are missing a comma on line 189 of src/staticStyles.js

As it happens, we don’t want that order stuff in staticStyles.js. Your updates to dynamicStyles.js are enough to support this module.

Regarding the negativeMargin stuff, I think we want to start by removing anything related to negativeMargin, since negative margins will now be handled by the margin module. Then from there it is a case of mapping a class name to a config value.

Currently we have a margin module, and a negativeMargin module. When we encounter the class name -mt-1 we need to figure out which module it belongs to, and what the config key is. Right now the answer would be the negativeMargin module, and 1 is the config key. But we are removing the negativeMargin module. Instead the answer should now be: the margin module, with a config key of -1

DevanB commented 5 years ago

It won’t build because you are missing a comma on line 189 of src/staticStyles.js

Derp. Thanks for the extra eyes there 😄

As it happens, we don’t want that order stuff in staticStyles.js. Your updates to dynamicStyles.js are enough to support this module.

Oh, I assumed dynamicStyles was only handling the dynamic values (e.g. 1, 2, 3). Does it also handle first, last, and none?

I presumed this was the case. I'll see what I can do. I'm definitely learning how this macro works, so excuse all of my (dumb) questions!

bradlc commented 5 years ago

Oh, I assumed dynamicStyles was only handling the dynamic values (e.g. 1, 2, 3). Does it also handle first, last, and none?

first, last, and none are dynamic, in the sense that they are defined in the config. "Static" (in the context of this plugin) refers to class names that are constant. They aren’t affected by config and always apply the same styles. For example there is no way to change what flex means. It’s always display: flex.

I presumed this was the case. I'll see what I can do. I'm definitely learning how this macro works, so excuse all of my (dumb) questions!

Hah they aren’t dumb!

DevanB commented 5 years ago

Excellent explanation, thanks 😄

Alright, I feel like I'm in over my head now, but I believe I have a solution!

if (prefix) {
  if (state.isDev) {
    state.shouldImportConfig = true
  }
  let key = className.substr(prefix.length + 1)
  if (key === '') key = 'default'

  if (prefix.startsWith('-')) key = '-' + key

  let obj

If someone adds a - to something that isn't supported, they get an error (thanks to your latest update!), otherwise we set the key to negative itself.

DevanB commented 5 years ago

Odd, I can't request a review, but I believe this is in working order @bradlc

bradlc commented 5 years ago

Thanks @DevanB !