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

Remove deprecated CSS tooling. Introduce postcss. #1113

Merged
merged 7 commits into from
Dec 15, 2022

Conversation

stevepolitodesign
Copy link
Contributor

@stevepolitodesign stevepolitodesign commented Dec 14, 2022

🚧 Work in progress

✅ To-do

  • Remove sprockets except for API mode
  • Remove obsolete tests
  • Add test coverage for the new css option
  • Add test coverage for the Gems
  • Use correct script tags #1111
  • Manually test that the generated app loads new CSS and JS

This was referenced Dec 14, 2022
@stevepolitodesign
Copy link
Contributor Author

We might also want to explore removing the following Gems from templates/Gemfile.erb and seeing if the build command adds those gems automatically.

gem "jsbundling-rails"
gem "cssbundling-rails"

lib/suspenders.rb Outdated Show resolved Hide resolved
@DoodlingDev
Copy link
Contributor

something to consider tomorrow (today? when are you reading this anyway?)

deleting template/descriptions/stylesheet_base.md
bourbon in templates/application.scss

the tests that are failing because they expect to see bourbon in the gemfile.. I think we can just get rid of those, I thought briefly about reversing their polarity to test that bourbon and bitters aren't installed but I talked myself out of it.

@stevepolitodesign stevepolitodesign force-pushed the 1112-simplify-asset-dependencies branch 2 times, most recently from 07483b5 to 4eb5ee8 Compare December 15, 2022 13:01
@stevepolitodesign stevepolitodesign force-pushed the 1112-simplify-asset-dependencies branch from 4eb5ee8 to 96f0284 Compare December 15, 2022 13:02
Comment on lines +10 to +16
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1, shrink-to-fit=no" />
<%%= csrf_meta_tags %>
<%%= csp_meta_tag %>

<%%= stylesheet_link_tag "application", "data-turbo-track": "reload" %>
<%%= javascript_include_tag "application", "data-turbo-track": "reload", defer: true %>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generated a new Rails app without suspenders, and this is what was in application.html.erb.

rails g new_app -c=postcss

@@ -28,7 +25,8 @@ gem "tzinfo-data", platforms: [:mingw, :x64_mingw, :mswin, :jruby]
<%# gem "webpacker" %>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should just remove this now.

Comment on lines +271 to +280
it "adds and configures a bundler strategy for css and js" do
gemfile = read_project_file("Gemfile")

expect(gemfile).to match(/bourbon/)
expect(gemfile).to match(/cssbundling-rails/)
expect(gemfile).to match(/jsbundling-rails/)
expect(File).to exist("#{project_path}/postcss.config.js")
expect(File).to exist("#{project_path}/package.json")
expect(File).to exist("#{project_path}/bin/dev")
expect(File).to exist("#{project_path}/app/assets/stylesheets/application.postcss.css")
expect(File).to exist("#{project_path}/app/javascript/application.js")
Copy link
Contributor Author

@stevepolitodesign stevepolitodesign Dec 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ensures the --css=postcss flag did its job.

A future commit should explore if we can remove the Gem all together.
@stevepolitodesign stevepolitodesign marked this pull request as ready for review December 15, 2022 13:49
Comment on lines +10 to +11
<% if options[:api] %>
gem "sprockets", "< 4"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need this for API only mode #1117

@stevepolitodesign stevepolitodesign merged commit 5c27b46 into support_rails_7 Dec 15, 2022
@stevepolitodesign stevepolitodesign deleted the 1112-simplify-asset-dependencies branch December 15, 2022 17:19
This was referenced Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants