Tailwind Setup: Use fully specified paths by Tobbe · Pull Request #1648 · redwoodjs/graphql

Tobbe · 2021-01-15T07:27:16Z

github-actions · 2021-01-15T07:32:04Z

📦 PR Packages

Click to Show Package Download Links

Install this PR by running yarn rw upgrade --pr 1648:0.23.0-0a14ca4

Tobbe · 2021-01-15T07:47:00Z

tobbe@XPS9550 MINGW64 ~/dev/redwood/tailwindpathspr/web/src/pages/TestPage
$ yarn rw setup tailwind
yarn run v1.22.4
$ C:\Users\tobbe\dev\redwood\tailwindpathspr\node_modules\.bin\rw setup tailwind
  √ Installing packages...
  √ Configuring PostCSS...
  √ Initializing Tailwind CSS...
  √ Adding imports to index.css...
  √ One more thing...

           Tailwind configured with "upcoming change" opt-in enabled

           See this doc for info: https://tailwindcss.com/docs/upcoming-changes

Done in 95.67s.

Tobbe · 2021-01-15T08:24:01Z

Fixing this uncovered another bug with the tailwind setup. --force isn't working as it should.

tobbe@XPS9550 MINGW64 ~/dev/redwood/tailwindpaths/web/src/pages/TestPage
$ yarn rw setup tailwind --force
yarn run v1.22.4
$ C:\Users\tobbe\dev\redwood\tailwindpaths\node_modules\.bin\rw setup tailwind --force
  √ Installing packages...
  √ Configuring PostCSS...
  × Initializing Tailwind CSS...
    → info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
    Adding imports to index.css...
    One more thing...
Command failed with exit code 1: yarn tailwindcss init
   � tailwind.config.js already exists.
error Command failed with exit code 1.
$ C:\Users\tobbe\dev\redwood\tailwindpaths\node_modules\.bin\tailwindcss init

   @tailwindcss/postcss7-compat 2.0.2


info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Done in 99.95s.

I'll try to fix that once this PR is merged

thedavidprice · 2021-01-16T19:27:56Z

Fixing this uncovered another bug with the tailwind setup. --force isn't working as it should.

@Tobbe please do take a look at the original PR addressing --force with this command. I seem to recall needing to fail the overwrite if/when there was a case where manual changes to the tailwind config existed. Maybe? Anyway, it might not be more output messaging confusion vs. command not working. Will defer to you.

thedavidprice

LGTM! Let me know if you need a Mac use to test first before merging. Otherwise 🚀

Tobbe · 2021-01-17T10:23:37Z

Let me know if you need a Mac use to test first before merging.

That would be great! This is how I tested it:

yarn create redwood-app twpath
cd twpath
yarn rw upgrade --pr 1648:0.23.0-8487f8c
yarn rw g page test
cd web/src/pages/TestPage
yarn rw setup tailwind

Tobbe · 2021-01-17T11:21:17Z

I went back and took another look at this. What a mess the tailwind installation is! I have two installations! One in ~/.../twpaths/node_modules and another one in ~/.../twpaths/web/node_modules

So if I'm running the setup command in ~/.../twpaths it will use the installation in ~/.../twpaths/node_modules, and files will be created relative to that path. If I'm somewhere inside web/, like ~/.../twpaths/web/src/pages, it will use the installation in ~/.../twpaths/web/node_modules and files will be created relative to that path instead.

Tobbe · 2021-01-17T12:40:15Z

please do take a look at the original PR addressing --force

You mean this one? #1301 I don't see anything in there about blocking --force. What am I missing?

Here's my understanding of the issue I found:

Scenario 1 (works)

~/.../twpath $ yarn rw setup tailwind

yarn tailwindcss init is executed. It creates ~/.../twpath/tailwind.config.js

At this point, if you were to run yarn tailwindcss init again it would fail, complaining about the existence of tailwind.config.js

Our setup command moves the config file to web/tailwind.config.js

~/.../twpath $ yarn rw setup tailwind --force

web/tailwind.config.js exists, but because --force is specified we continue
yarn tailwindcss init is executed. It doesn't find any config file (because we previously moved it into web/), and so it happily creates a new one in ~/.../twpath. We will move it into web/, overwriting the existing file

Scenario 2 (fails)

~/.../twpath/web/src/pages/TestPage $ yarn rw setup tailwind

yarn tailwindcss init is executed. cwd() is now ~/.../twpath/web, so it creates ~/.../twpath/web/tailwind.config.js

At this point, if you were to run yarn tailwindcss init again it would fail, complaining about the existence of tailwind.config.js

Our setup command moves the config file to web/tailwind.config.js (which is a no-op, since that's where it already is)

~/.../twpath/web/src/pages/TestPage $ yarn rw setup tailwind --force

web/tailwind.config.js exists, but because --force is specified we continue
yarn tailwindcss init is executed. It now finds web/tailwind.config.js because cwd() is ~/.../twpath/web. The command fails, complaining about the existing config file.

packages/cli/src/commands/setup/tailwind/tailwind.js

Tobbe · 2021-01-18T23:09:57Z

@thedavidprice If you could run a quick test on your (mac) computer, that would be awesome!

yarn create redwood-app twpath
cd twpath
yarn rw upgrade --pr 1648:0.23.0-7e8ddfc
yarn rw g page test
cd web/src/pages/TestPage
yarn rw setup tailwind

thedavidprice

@Tobbe this is working (almost) 💯 on Mac! From root, install and --force work correctly. From a sub-directory, install works correctly but --force runs into an issue with Tailwind yarn tailwindcss init that fails if tailwind.config.js exists. This is the reason why we added the "Sync ..." step that runs yarn install --check-files, which avoids Tailwind throwing. Here's the error:

Command failed with exit code 1: yarn tailwindcss init
   🚫 tailwind.config.js already exists.
error Command failed with exit code 1.
$ /Users/price/Repos/xx-delete/node_modules/.bin/tailwindcss init

   @tailwindcss/postcss7-compat 2.0.2

To run --force successfully from any directory, I think we need to set the Execa working directory to root on this line:
https://github.com/redwoodjs/redwood/blob/ab1b7229f40dd6668244f1bb2d866ed8829de806/packages/cli/src/commands/setup/tailwind/tailwind.js#L89

peterp · 2021-01-19T06:58:53Z

@Tobbe Thinking out loud here, maybe we can pass cwd to execa and always make it use getPaths().web.base, then we can remove the "move" code, since we would always init the tailwind in the right place.

Tobbe · 2021-01-19T09:30:20Z

@Tobbe this is working (almost) 💯 on Mac!

Thanks for the thorough testing. You're seeing the exact same thing as I'm seeing on Windows. You can compare your output with what I posted in a previou comment #1648 (comment)

As I also said in that comment, my plan was to merge this fix, and then write a separate fix for the --force stuff. But I guess I can do it all here.

My plan for fixing --force was to just manually delete tailwind.config.js first, before running yarn tailwindcss init.

To run --force successfully from any directory, I think we need to set the Execa working directory to root on this line:
await execa('yarn', ['install', '--check-files'])
@thedavidprice We get "tailwind.config.js already exists." from tailwindcss init. Will yarn install --check-files help with that?

thedavidprice · 2021-01-19T16:13:50Z

Ah, roger that @Tobbe I (falsely) assumed you were ready to go and skipped over:

You're seeing the exact same thing as I'm seeing on Windows. You can compare your output with what I posted in a previous comment #1648 (comment)

Yes, yarn install --check-files handles the "previously installed" error. This is why --force works when run from root. So my vote would be to go with @peterp suggestion to set cwd.

Thanks! 'Cause this week you are:

Tobbe · 2021-01-19T17:33:24Z

This is why --force works when run from root.

I don't think that's true. The reason it works from root is that there is no tailwind.config.js file there that yarn tailwindcss init can find. You can try it yourself by manually running yarn tailwindcss init in root. It will work just fine. Then go in to web/ and run it from there, and it will give you that same error message as you get when running yarn rw setup tailwind --force

my vote would be to go with @peterp suggestion to set cwd

I will. But I don't think it'll help with --force (Give me a few minutes and we'll know for sure 😉)

Tobbe · 2021-01-19T18:30:44Z

@thedavidprice As I expected, now, when I specify cwd to be web/ it doesn't even work with --force in root anymore, because now yarn tailwindcss init finds tailwind.config.js in web/, even if we stand in root.

tobbe@XPS9550 MINGW64 ~/dev/redwood/tailwindpathspr
$ yarn rw setup tailwind --force
yarn run v1.22.4
$ C:\Users\tobbe\dev\redwood\tailwindpathspr\node_modules\.bin\rw setup tailwind --force
  √ Installing packages...
  √ Configuring PostCSS...
  × Initializing Tailwind CSS...
    → info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
    Adding imports to index.css...
    One more thing...
Command failed with exit code 1: yarn tailwindcss init
   � tailwind.config.js already exists.
error Command failed with exit code 1.
$ C:\Users\tobbe\dev\redwood\tailwindpathspr\node_modules\.bin\tailwindcss init

   @tailwindcss/postcss7-compat 2.0.2


info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Done in 115.82s.

Tobbe · 2021-01-19T18:53:08Z

Now that I delete the config file first it works

tobbe@XPS9550 MINGW64 ~/dev/redwood/tailwindpathspr
$ yarn rw setup tailwind --force
yarn run v1.22.4
$ C:\Users\tobbe\dev\redwood\tailwindpathspr\node_modules\.bin\rw setup tailwind --force
  √ Installing packages...
  √ Configuring PostCSS...
  √ Initializing Tailwind CSS...
  ↓ Adding imports to index.css... [skipped]
    → Imports already exist in index.css
  √ One more thing...

           Tailwind configured with "upcoming change" opt-in enabled

           See this doc for info: https://tailwindcss.com/docs/upcoming-changes

Done in 113.02s.

Tobbe · 2021-01-19T18:57:47Z

Next problem to handle: Paths with spaces!

tobbe@XPS9550 MINGW64 ~/dev/redwood/tail wind pathspr
$ yarn rw setup tailwind --force
yarn run v1.22.4
$ "C:\Users\tobbe\dev\redwood\tail wind pathspr\node_modules\.bin\rw" setup tailwind --force
  √ Installing packages...
  √ Configuring PostCSS...
  × Initializing Tailwind CSS...
    → spawn C:\WINDOWS\system32\cmd.exe ENOENT
    Adding imports to index.css...
    One more thing...
Command failed with ENOENT: yarn tailwindcss init
spawn C:\WINDOWS\system32\cmd.exe ENOENT
Done in 102.09s.

Tobbe · 2021-01-19T19:32:37Z

🎉

tobbe@XPS9550 MINGW64 ~/dev/redwood/tail wind pathspr
$ yarn rw setup tailwind --force
yarn run v1.22.4
$ "C:\Users\tobbe\dev\redwood\tail wind pathspr\node_modules\.bin\rw" setup tailwind --force
  √ Installing packages...
  √ Configuring PostCSS...
  √ Initializing Tailwind CSS...
  ↓ Adding imports to index.css... [skipped]
    → Imports already exist in index.css
  √ One more thing...

           Tailwind configured with "upcoming change" opt-in enabled

           See this doc for info: https://tailwindcss.com/docs/upcoming-changes

Done in 128.52s.

tobbe@XPS9550 MINGW64 ~/dev/redwood/tail wind pathspr
$ cd web/src/pages/

tobbe@XPS9550 MINGW64 ~/dev/redwood/tail wind pathspr/web/src/pages
$ yarn rw setup tailwind --force
yarn run v1.22.4
$ "C:\Users\tobbe\dev\redwood\tail wind pathspr\node_modules\.bin\rw" setup tailwind --force
  √ Installing packages...
  √ Configuring PostCSS...
  √ Initializing Tailwind CSS...
  ↓ Adding imports to index.css... [skipped]
    → Imports already exist in index.css
  √ One more thing...

           Tailwind configured with "upcoming change" opt-in enabled

           See this doc for info: https://tailwindcss.com/docs/upcoming-changes

Done in 118.56s.

Works with spaces in the path, and both at root level and deep inside web/

thedavidprice · 2021-01-20T19:01:11Z

As I expected, now, when I specify cwd to be web/ it doesn't even work with --force in root anymore...

Ah, understood. I was suggesting to set the cwd to be root. All good.

There were two reasons (if I recall correctly) why we had to run the --check-file, which were both related to tailwind already being installed. I'm going to re-test now --> there's a chance with you new code we don't need the "Sync" step and can remove the --check-file process, which takes way too long anyway.

Testing now.

thedavidprice · 2021-01-20T19:50:21Z

Testing looks good ✅

@Tobbe We don't need this step anymore. Can you remove starting L82?

{
            title: 'Sync yarn.lock and node_modules',
            task: async () => {
              /**
               * Sync yarn.lock file and node_modules folder.
               * Refer https://github.com/redwoodjs/redwood/issues/1301 for more details.
               */
              await execa('yarn', ['install', '--check-files'])
            },
          },

Tobbe · 2021-01-20T21:50:12Z

@thedavidprice I actually already tried that, but just by manually editing the setup command inside node_modules. This is what I got

$ yarn rw setup tailwind --force
yarn run v1.22.4
$ "C:\Users\tobbe\dev\redwood\tail wind pathspr\node_modules\.bin\rw" setup tailwind --force
  √ Installing packages...
  √ Configuring PostCSS...
  × Initializing Tailwind CSS...
    → info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
    Adding imports to index.css...
    One more thing...
Command failed with exit code 1: yarn tailwindcss init
error Command "tailwindcss" not found.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Done in 29.75s.

But since you say it'll work without it I'm going to build a proper new package to test with

Tobbe · 2021-01-20T22:12:05Z

Same error with the PR package

tobbe@XPS9550 MINGW64 ~/dev/redwood/tail wind pathspr
$ yarn rw setup tailwind --force
yarn run v1.22.4
$ "C:\Users\tobbe\dev\redwood\tail wind pathspr\node_modules\.bin\rw" setup tailwind --force
  √ Installing packages...
  √ Configuring PostCSS...
  × Initializing Tailwind CSS...
    → info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
    Adding imports to index.css...
    One more thing...
Command failed with exit code 1: yarn tailwindcss init
error Command "tailwindcss" not found.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Done in 25.39s.

thedavidprice · 2021-01-20T22:20:35Z

Super weird. I tested first locally by removing it from my installed package.

Well then, let's ship this as you had it! Thanks for trying it out.

And, oh my, it's unreal how much time and attention this specific Setup command has required. This is either round 3 or 4 of a deep dive. We must really ❤️ Tailwind...

did not work on Windows ¯_(ツ)_/¯

Tobbe · 2021-01-20T22:26:59Z

We must really ❤️ Tailwind...

Haha. I don't even use tailwind. I just wanted to do the community a service 🤷‍♂️

I'm running the latest PR package again, on a fresh install, just to rule out my manual changes as the source of the error. Will report back in a bit.

Tobbe · 2021-01-20T22:29:17Z

First run without yarn install --check-files works fine. Second one (with --force) throws the error

tobbe@XPS9550 MINGW64 ~/dev/redwood/twpath
$ yarn rw setup tailwind
yarn run v1.22.4
$ C:\Users\tobbe\dev\redwood\twpath\node_modules\.bin\rw setup tailwind
  √ Installing packages...
  √ Configuring PostCSS...
  √ Initializing Tailwind CSS...
  √ Adding imports to index.css...
  √ One more thing...

           Tailwind configured with "upcoming change" opt-in enabled

           See this doc for info: https://tailwindcss.com/docs/upcoming-changes

Done in 27.78s.

tobbe@XPS9550 MINGW64 ~/dev/redwood/twpath
$ yarn rw setup tailwind --force
yarn run v1.22.4
$ C:\Users\tobbe\dev\redwood\twpath\node_modules\.bin\rw setup tailwind --force
  √ Installing packages...
  √ Configuring PostCSS...
  × Initializing Tailwind CSS...
    → info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
    Adding imports to index.css...
    One more thing...
Command failed with exit code 1: yarn tailwindcss init
error Command "tailwindcss" not found.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Done in 21.98s.

Fixes redwoodjs#1643

thedavidprice · 2021-01-21T00:13:07Z

🚀

thedavidprice approved these changes Jan 16, 2021

View reviewed changes

Tobbe force-pushed the tobbe-tw-setup-paths branch 2 times, most recently from 1f12f1d to b45bad0 Compare January 17, 2021 12:29

peterp reviewed Jan 17, 2021

View reviewed changes

packages/cli/src/commands/setup/tailwind/tailwind.js Outdated Show resolved Hide resolved

peterp reviewed Jan 17, 2021

View reviewed changes

packages/cli/src/commands/setup/tailwind/tailwind.js Outdated Show resolved Hide resolved

peterp approved these changes Jan 17, 2021

View reviewed changes

peterp added the next release label Jan 18, 2021

peterp added this to the next release milestone Jan 18, 2021

Tobbe force-pushed the tobbe-tw-setup-paths branch 3 times, most recently from a94040c to dab69e8 Compare January 18, 2021 10:29

thedavidprice previously requested changes Jan 19, 2021

View reviewed changes

Tobbe force-pushed the tobbe-tw-setup-paths branch 2 times, most recently from 2a4bd73 to 17194f6 Compare January 19, 2021 18:21

Tobbe force-pushed the tobbe-tw-setup-paths branch from 17194f6 to b4398ac Compare January 19, 2021 18:37

Tobbe force-pushed the tobbe-tw-setup-paths branch from b4398ac to b8edd16 Compare January 19, 2021 19:11

peterp approved these changes Jan 19, 2021

View reviewed changes

Tobbe force-pushed the tobbe-tw-setup-paths branch from 363379a to 0a20ef9 Compare January 20, 2021 21:59

Tailwind Setup: Use fully specified paths

0a14ca4

Fixes redwoodjs#1643

Tobbe force-pushed the tobbe-tw-setup-paths branch from 0a20ef9 to 0a14ca4 Compare January 20, 2021 22:33

Tobbe merged commit 0d62c8f into redwoodjs:main Jan 20, 2021

thedavidprice mentioned this pull request Jan 21, 2021

Add option to setup TailwindUI with TailwindCSS #1408

Closed

Conversation

Tobbe commented Jan 15, 2021

Uh oh!

github-actions bot commented Jan 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📦 PR Packages

Uh oh!

Tobbe commented Jan 15, 2021

Uh oh!

Tobbe commented Jan 15, 2021

Uh oh!

thedavidprice commented Jan 16, 2021

Uh oh!

thedavidprice left a comment

Choose a reason for hiding this comment

Uh oh!

Tobbe commented Jan 17, 2021

Uh oh!

Tobbe commented Jan 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Tobbe commented Jan 17, 2021

Here's my understanding of the issue I found:

Uh oh!

Uh oh!

Uh oh!

Tobbe commented Jan 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thedavidprice left a comment

Choose a reason for hiding this comment

Uh oh!

peterp commented Jan 19, 2021

Uh oh!

Tobbe commented Jan 19, 2021

Uh oh!

thedavidprice commented Jan 19, 2021

Uh oh!

Tobbe commented Jan 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Tobbe commented Jan 19, 2021

Uh oh!

Tobbe commented Jan 19, 2021

Uh oh!

Tobbe commented Jan 19, 2021

Uh oh!

Tobbe commented Jan 19, 2021

Uh oh!

thedavidprice commented Jan 20, 2021

Uh oh!

thedavidprice commented Jan 20, 2021

Uh oh!

Tobbe commented Jan 20, 2021

Uh oh!

Tobbe commented Jan 20, 2021

Uh oh!

thedavidprice commented Jan 20, 2021

Uh oh!

Tobbe commented Jan 20, 2021

Uh oh!

Tobbe commented Jan 20, 2021

Uh oh!

thedavidprice commented Jan 21, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Jan 15, 2021 •

edited

Loading

Tobbe commented Jan 17, 2021 •

edited

Loading

Tobbe commented Jan 18, 2021 •

edited

Loading

Tobbe commented Jan 19, 2021 •

edited

Loading