Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

modify generated tailwind config to opt-in to upcoming change #1179

Merged

Conversation

forresthayes
Copy link
Contributor

@forresthayes forresthayes commented Sep 18, 2020

resolves #1064

@peterp, I noticed the redwood tailwind utility generates a config with a removeGeneratedGapUtilities flag that is commented out. Was the commented flag present when you opened the issue? If not, would you rather leave it as is?

There is also another upcoming-change generated flag regarding purging layers

For now, my PR only uncomments the flag for removing the gap utilities.

Just let me know what you think is best, Thanks!

@peterp
Copy link
Contributor

peterp commented Sep 18, 2020

I'm not entirely sure why it was commented out, maybe @jtoar knows a bit about the history for this?

So the idea with this pull-request is that you run it again on your existing project and it'll uncomment that flag? Maybe we should just make it so that removeGeneratedGapUtilities is uncommented to begin with?

@jtoar
Copy link
Contributor

jtoar commented Sep 18, 2020

@peterp The line must be commented out by default—all the tailwind generator does right now is run the command to generate the config:

yarn tailwindcss init

and what gets initialized must be changing with the versions. So @forresthayes, if these changes are what's recommended, I'd say we opt into them!

@peterp it looks like this PR makes it so that removeGeneratedGapUtilities will be uncommented to begin with (can confirm @forresthayes?).

@forresthayes
Copy link
Contributor Author

@jtoar @peterp

I updated my PR to opt-in to all upcoming tailwind config changes. But you guys know best 😀, if this approach is inefficient or unnecessary, we can move on! I just thought I'd give it a shot.

By the way, I explored using execa to run a sed command, but I think the options would be OS dependent.

@peterp
Copy link
Contributor

peterp commented Sep 23, 2020

@forresthayes I like the approach you're thinking about, but I'm worried that this may be too much for Redwood to handle, and is better dealt with by tailwind.

Maybe a better approach is to nudge the developer towards reading the future docs and manually opting in them if they choose to do so?

I'm not 100% sure.

@peterp
Copy link
Contributor

peterp commented Sep 30, 2020

Hey @forresthayes I saw a tweet a few days ago that tailwindcss init can actually generate the postcss config itself - could we close this one off and then do that instead?

@thedavidprice
Copy link
Contributor

@forresthayes See my review request re: next step and moving this to the new setup command directory.

I'll get this merged asap once you have a chance to update. Thanks!

@forresthayes
Copy link
Contributor Author

@thedavidprice

I moved the code that opts-in to future Tailwind changes into the new setup tailwind script per your request, but, looking at Peter's comments above, I totally understand if this approach goes too far or is liable to break. 🤷‍♂️

@thedavidprice
Copy link
Contributor

This works great @forresthayes I'm merging now and will follow up with a quick addition to add info in the final output about the enabled opt-in.

🚀

@thedavidprice thedavidprice merged commit 4bed1aa into redwoodjs:main Oct 4, 2020
@thedavidprice thedavidprice added this to the Next release milestone Oct 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "removeDeprecatedGapUtilities: true" to generated tailwind config
4 participants