lib/install/tailwindcss.rb assumes newline at end of app/assets/config/manifest.js

[litchk]

If there is no newline at the end of app/assets/config/manifest.js, append_to_file in the following codeblock will add //= link_tree ../builds to the end of the last line, breaking the build with a process_link_tree_directive error:

# tailwindcss-rails/lib/install/tailwindcss.rb:23-25

if (sprockets_manifest_path = Rails.root.join("app/assets/config/manifest.js")).exist?
  append_to_file sprockets_manifest_path, %(//= link_tree ../builds\n)
end

I think the simplest fix would be to add a newline to the start of the line added by append_to_file, as lib/install/tailwindcss.rb does for the append_to_file call targeting .gitignore:

# tailwindcss-rails/lib/install/tailwindcss.rb:27-29

if Rails.root.join(".gitignore").exist?
  append_to_file(".gitignore", %(\n/app/assets/builds/*\n!/app/assets/builds/.keep\n))
end

This will result in extra whitespace sometimes in app/assets/config/manifest.js, but that doesn't appear to be semantically significant.

If the maintainers agree with this approach, I'd be happy to open a PR to fix. Thanks for reading and for the work you do on this great gem ❤️

[flavorjones]

@litchk Thanks for reporting this! Yes, that approach seems fine to me and I'd be very grateful if you opened a pull request. Thank you!

[litchk]

Sure thing, and will do.

Do you have any opinions on how to test the fix within the context of the current test setup? lib/install/tailwindcss.rb is currently only tested by test/integration/user_journey_test.sh.

I think ideally a test would check that a app/assets/config/manifest.js file both with and without a newline at the end is properly formed after running lib/install/tailwindcss.rb. So my thought is to basically have 2 or 3 tests:

1. Run rails new, replace app/assets/config/manifest.js with a dummy file without a newline at the end, run lib/install/tailwindcss.rb, check that app/assets/config/manifest.js looks OK
2. Run rails new, replace app/assets/config/manifest.js with a dummy file with a newline at the end, run lib/install/tailwindcss.rb, check that app/assets/config/manifest.js looks OK
3. Run rails new, don't touch app/assets/config/manifest.js, run lib/install/tailwindcss.rb, check that app/assets/config/manifest.js looks OK

(3) doesn't really do anything different, but I like the idea of testing against what rails new really outputs. Ideally I would just generate a Rails app once for all the tests, for speed's sake.

I could follow the model of test/integration/user_journey_test.sh for this or do it within Test::Unit.

By the way, I think I should fix/test the same thing for append_to_file "Procfile.dev", "css: bin/rails tailwindcss:watch\n".

Let me know if you have any opinions here, if not can just make a call and you can let me know your thoughts on the result.

[flavorjones]

@litchk Thanks for asking about testing ... this feels like enough of an edge case, and integration testing with the user journey is challenging enough (and takes SO LONG on windows!🙄), that I'm OK skipping test coverage here.