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

Suspenders 3.0.0 #1135

Merged
merged 71 commits into from May 10, 2024
Merged

Suspenders 3.0.0 #1135

merged 71 commits into from May 10, 2024

Conversation

stevepolitodesign
Copy link
Contributor

@stevepolitodesign stevepolitodesign commented Oct 17, 2023

Rewrite Suspenders as a Rails engine in an effort to achieve our goals while improving the developer experience, reducing technical debt and auditing our existing generators. The goal is to leverage as much of rails new as possible, and only deviate where necesary.

This removes the suspenders system executable in favor of application templates. Moving forwared, there will be two ways to use Suspenders.

With New Applications

rails new <my_app> \
 --skip-test \
 -d=postgresql \
 -m=https://raw.githubusercontent.com/thoughtbot/suspenders/main/lib/install/web.rb

With Existing Applications

group :development, :test do
  gem "suspenders", github: "thoughtbot/suspenders"
end

How to review this pull request

This long running feature branch is built upon a commit for each new generator. For more contenxt, I recommend looking at the commit history. I tried to make each pull request as detailed as possible.

To get a sense if everything works in concert, run the following:

rails new app_name \
 --skip-test \
 -d=postgresql \
 -m=https://raw.githubusercontent.com/thoughtbot/suspenders/suspenders-3-0-0/lib/install/web.rb

cd app_name
bin/setup

bundle exec rake
yarn run lint

Then I recommend scaffolding out a resource and adding a system test too.

bin/rails g scaffold post title
bin/rails suspenders:db:migrate
require "rails_helper"

RSpec.describe "Posts", type: :system do
  it "passes" do
    visit root_path

    expect(page).to have_text "Posts"
  end
end

Contributing

The 3.0.0 release is focused on all generators needed for a new Rails application. Anything that is not necessary for a new application will be added in a subsequent release. Additionally, a future release will need to account for existing applications in a way that does not re-install gems.

This branch will remain as a draft until all to-dos are complete.

  1. Branch off of this branch.
  2. Run ./bin/seup. Note, this you'll need to use Ruby 3.0.0 or higher.
  3. Create and squash and merge pull-request with a generator, updated README, and updated NEWS against this branch.
  4. Repeat.

If something needs to change about the core branch (CI improvements, setup
improvements, directory structure) commit directly to this branch and rebase

git checkout suspenders-3-0-0
git pull --rebase
git checkout suspenders-3-0-0-feature-branch
git rebase suspenders-3-0-0

Status

Generator Status
AccessibilityGenerator Complete
AdvisoriesGenerator Complete
AnalyticsGenerator Won't Do
AppGenerator To Do (See "App Generators" below.)
CiGenerator Complete
DbOptimizationsGenerator Won't Do (Consider using strict_loading config instead)
FactoriesGenerator Complete
FormsGenerator Won't Do
InlineSvgGenerator Complete
JobsGenerator Complete (Needs to conditionally update Procfile.dev. Look at cssbundling-rails for inspiration.)
JsDriverGenerator Complete (Only for RSpec apps)
JsonGenerator Won't Do (I think this is only necessary for API only apps)
LintGenerator Complete (Modify by removing Hound. Could also benefit from ERB linting)
ProfilerGenerator Won't Do (This ships with Rails, but commented out.)
RunnerGenerator Won't Do (Procfile.dev depends on several factors, including how you build assets, and what job runner is being used. )
StaticGenerator Won't Do (Not needed for all apps. Could make for a one-off generator.)
StylelintGenerator Complete
StylesheetBaseGenerator Complete
TestingGenerator Complete (Only for RSpec apps. Consider renaming.)
ViewsGenerator Complete

App Generators

These were extracted from AppGenerator. Some generators were left out because they are no longer relevant.

Should we break these up into smaller generators, or just consolidate everything into an AppGenerator?

Generator Status
Development::EnvironmentGenerator Complete
setup_production_environment Complete
configure_app Complete. (Extracted into suspenders:email and lib/tasks/suspenders.rake. We did not do configure_time_formats because that may end up beind dead code, and is not neccersary for new apps.)
copy_miscellaneous_files Won't Do(config/initializers/errors.rb isn't mission critical, and the JSON encoding for timestamps should probably use the Rails default, which is milliseconds)
setup_database Needs more work (Except create_database, do we need the database.yml overwrite? It is just for Heroku?)
SingleRedirect Won't Do (This can be an infrastructure decision.)
CompressionGenerator Won't Do (This can be an infrastructure decision.)
ForceTlsGenerator Doing
EmailGenerator Complete (also take ActionMailer configuration from the existing configure_app

New Generators Worth Considering

These could be added as part of the initial release, or as a subsequent minor release.

Generator Status
dev:prime Complete
rake Complete
setup Complete
suspenders:db:migrate Complete
suspenders:json Needs Work. Maybe this is a good candidated for a future suspenders:install:api.
suspenders:authorization Needs Work. I think we need to establish an authorization opinion first.
suspenders:prerequisites Complete
suspenders:templates Doing

To do

Once all is complete, rebase and merge to keep commit history.

@stevepolitodesign stevepolitodesign force-pushed the suspenders-3-0-0 branch 2 times, most recently from 1cfee95 to 06c8d27 Compare October 17, 2023 11:38
RUBY_VERSION_RANGE = [minimum_ruby_version, maximum_ruby_version].freeze
VERSION = "20230113.0".freeze
VERSION = "3.0.0".freeze
RAILS_VERSION = "~> 7.0".freeze
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we're removing the executable, we don't need to be on the latest version of Rails. My thinking was, I don't want to prevent someone not on >= 7.1 to be able to use the gem since it's just generators.

However, the only downside to that is if a generator uses a feature from >= 7.1, that feature won't work.

Copy link
Member

@mike-burns mike-burns left a comment

Choose a reason for hiding this comment

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

Looks like you've just begun. I'll watch as you commit more!

MIT-LICENSE Outdated Show resolved Hide resolved
In an effort to improve the developer experience and reduce technical
debt, we replace the existing project with the output from running:

```
rails plugin new suspenders
```

This will allow us to use existing [testing helpers][] provided by Rails.
Because of this, we deliberately switch from RSpec to MiniTest.

Additionally, we move away from [calver][]. Prior to that the latest major
version was `1`. This feels like a good opportunity to go back to `semver` and
start fresh with `3.0.0`

[testing helpers]: https://edgeguides.rubyonrails.org/generators.html#testing-generators
[calver]: https://calver.org
All generators will affect the "test" app located in `test/dummy`.
Although the "test" app contains most files for a Rails application, it
does not contain everything. This means during the setup phase of our
tests, we'll need to create files to ensure the generator is working
correctly.

These helpers aim to improve the developer experience by creating a
simple API to create and delete files in the "test" app.

It should be noted that any files and directories created in the setup
won't be removed automatically after each test, so we introduce
additional helpers to be called during the [teardown][] callback.

[teardown]: https://github.com/rails/rails/blob/00dfa109d6a5f4fb13d745f83d5cc779eba3b352/railties/lib/rails/generators/testing/setup_and_teardown.rb#L13
Use [mocha][] for mocking and stubbing. Specifically, this will aid with
testing that we correctly install a gem with bundler in our generators.

[mocha]: https://github.com/freerange/mocha
stevepolitodesign and others added 16 commits November 3, 2023 15:37
Ported over from #1105

Installs [capybara_accessibility_audit] and
[capybara_accessible_selectors].

`./bin/rails g suspenders:accessibility`

Introduces `Suspenders::Generators::APIAppUnsupported` module for
generators that cannot be run in an [API only][] application.

This uses a [concern][] to ensure we raise an error before the generator
including the module invokes any of its methods.

[capybara_accessibility_audit]: https://github.com/thoughtbot/capybara_accessibility_audit
[capybara_accessible_selectors]: https://github.com/citizensadvice/capybara_accessible_selectors
[API only]: https://guides.rubyonrails.org/api_app.html
[concern]: https://api.rubyonrails.org/classes/ActiveSupport/Concern.html
This is a work around for an [issue][] with `vim-test`. Borrowed an
[existing][] `./bin/rails` configuration from an [Engine][], which is
very similar to a [plugin][]. The only difference is that we do not need
the `ENGINE_ROOT` setting.

[issue]: vim-test/vim-test#477
[existing]: https://github.com/rails/rails/blob/main/railties/lib/rails/generators/rails/plugin/templates/bin/rails.tt
[Engine]: https://guides.rubyonrails.org/engines.html
[plugin]: https://guides.rubyonrails.org/plugins.html
Ports the existing generator to Suspenders 3.0.
it's the same but shorter
Maintains functionally with the [existing generator][] while adding
support for the [default Rails test suite]. With this change, the
generator can be invoked on a Rails application that uses RSpec or the
[default Rails test suite][].

Adds generator which adds a test to lint all Factories in an effort to
improve developer experience.

Additionally, we remove the generation of the `dev:prime` task as we
felt that should be the responsibly of another generator.

[existing generator]: https://github.com/thoughtbot/suspenders/blob/main/lib/suspenders/generators/factories_generator.rb
[default Rails test suite]: https://guides.rubyonrails.org/testing.html
NEWS should only contain items relevant to the consumer. Although the
module improves the developer experience, it's not relevant to someone
using the gem.
Uses the [bundler-audit][] gem to update the local security database and
show any relevant issues with the app's dependencies. This generator is
only responsible for installing the gem and adding the Rake task.

The [original implementation][] was written in 2014, and is no longer
relevant. This is because the gem ships [with a Rake task][] that can be
set as the default task, which will be addressed in #1144

Also exposes `backup_file` and `restore_file` test helpers into the
public API.

[bundler-audit]: https://github.com/rubysec/bundler-audit
[original implementation]: e23157e
[with a Rake task]: https://github.com/rubysec/bundler-audit#rake-tasks
Configures applications to use [PostCSS][1] or [Tailwind][2] via
[cssbundling-rails][3]. Defaults to `PostCSS` with
[modern-normalize][8], with the option to override via `--css=tailwind`.
These options were pulled from the [supported list of options][4] in
Rails.

Also creates additional stylesheets if using PostCSS.

We choose to use [cssbundling-rails][4] instead of [dartsass-rails][5]
or [tailwindcss-rails][6] (or even just css) because we want to rely on
Node to process the CSS. Although we could have chosen to avoid using
Node altogether, we feel it's better to support it since we'll need it
for additional generators, like [StyleLintGenerator][7], and to support
[modern-normalize][8].

Updates `within_api_only_app` by allowing support to conditionally
comment out the api configuration. This provided and opportunity to
clean up existing setup steps.

[1]: https://postcss.org
[2]: https://tailwindcss.com
[3]: https://github.com/rails/cssbundling-rails
[4]: https://github.com/rails/rails/blob/438cad462638b02210fc48b700c29dcd0428a8b7/railties/lib/rails/generators/app_base.rb#L22
[5]: https://github.com/rails/dartsass-rails
[6]: https://github.com/rails/tailwindcss-rails
[7]: https://github.com/thoughtbot/suspenders/blob/main/lib/suspenders/generators/stylelint_generator.rb
[8]: https://github.com/sindresorhus/modern-normalize
[9]: https://tailwindcss.com/docs/functions-and-directives#layer
These modifications were not captured before merging the #1145,
presumably because of an issue with GitHub CLI.
Ports the existing generator to Suspenders 3.0.
In #1148 we introduce `stylelint`. However, the empty style sheets introduced in
#1145 are empty file violations. By adding comments, we avoid that error.

Note that we need to add these comments because there's no way to
automatically fix those violations with something like `prettier`.
Closes #1107
Closes #1143

Creates a holistic linting solution that covers JavaScript, CSS, Ruby
and ERB.

Introduces [scripts][] that leverage [@thoughtbot/eslint-config][],
[@thoughtbot/stylelint-config][] and [prettier][].

Also introduces `.prettierrc` based off of our [Guides][].

We need to pin `stylelint` and `@thoughtbot/stlyelint-config` to
specific versions to account for this [open issue][]. Unfortunately,
running `yarn run lint:stylelint` results in deprecation warnings, which
will need to be addressed separately.

[scripts]: https://docs.npmjs.com/cli/v6/using-npm/scripts
[@thoughtbot/eslint-config]: https://github.com/thoughtbot/eslint-config
[@thoughtbot/stylelint-config]: https://github.com/thoughtbot/stylelint-config
[prettier]: https://prettier.io
[Guides]: https://github.com/thoughtbot/guides/blob/main/javascript/README.md#formatting
[open issue]: thoughtbot/stylelint-config#46

Introduces `rake standard` which also runs `erblint` to lint ERB files
via [better_html][], [erb_lint][] and [erblint-github][].

[better_html]: https://github.com/Shopify/better-html
[erb_lint]: https://github.com/Shopify/erb-lint
[erblint-github]: https://github.com/github/erblint-github

A future commit will ensure these linting rules are run during CI. In an
effort to support that future commit, we ensure to run `yarn run
fix:prettier` and `bundle exec standard:fix_unsafely` once the generator
is complete. Otherwise, CI would fail because of linting violations.

We call `standard:fix_unsafely` since `standard:fix` returns an error
status code on new Rails applications. Running `standard:fix_unsafely`
fixes this issue and returns a success status code.

It should be noted that we deliberately permit this generator to be
invoked on API only applications, because those applications can still
contain views, like ones used for mailers. However, a future commit could
explore removing the JavaScript linters.

Also improves the developer experience by introducing `with_test_suite`
helper, allowing the caller to run the generator in an application using
minitest or RSpec.
Add necessary files to make the [plugin][] an [engine][], which automatically
loads Rake tasks located in `lib/tasks`. This means when the `suspenders` gem is
installed, the consumer can run `bundle exec rake suspenders:rake`, and any
future tasks.

Because `suspenders:lint` and `suspenders:advisories` may not necessarily have
been invoked, we need to check if those gems are installed.

[plugin]: https://guides.rubyonrails.org/plugins.html
[engine]: https://guides.rubyonrails.org/engines.html

Co-authored-by: Mike Burns <mburns@thoughtbot.com>
When we introduced #1148 we did not test it against applications that
invoked `suspenders:styles --css=tailwind`.
Configures flash messages, page titles via the [title][] gem, and sets
the document [lang][].

[title]: https://github.com/calebhearth/title
[lang]: https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/lang

It should be noted that running `rails gscaffold` generates [views][]
that contain the flash. Although we could have [overridden][] this
generator, that would risk drift between our overridden generator and
the one in Rails core.

Additionally, we decided to remove the `FlashesHelper` introduced in
[6c562b9][] since that is not a pattern
we currently use.

[views]: https://github.com/rails/rails/blob/main/railties/lib/rails/generators/erb/scaffold/templates/index.html.erb.tt
[overridden]: https://guides.rubyonrails.org/generators.html#overriding-rails-generator-templates
[6c562b9]: 6c562b9
Replaces the [default setup script][] provided by Rails. The trade-off
being that the implementation is simple, but the cost is we risk
drifting from Rails. After attempting to use [gsub_file][] and
[insert_into_file][], it felt simpler to just override the file
completely.

Conditionally call [dev:prime][] to seed development data on top of [seed][]
data necessary for production. A follow-up commit will re-introduce this task.

Additionally, we [setup the test environment][] to avoid issues with
asset compilation.

[default setup script]: https://github.com/rails/rails/blob/main/railties/lib/rails/generators/rails/app/templates/bin/setup.tt
[gsub_file]: https://rubydoc.info/gems/thor/Thor/Actions#gsub_file-instance_method
[insert_into_file]: https://rubydoc.info/gems/thor/Thor/Actions#insert_into_file-instance_method
[dev:prime]: https://thoughtbot.com/blog/priming-the-pump
[seed]: https://guides.rubyonrails.org/active_record_migrations.html#migrations-and-seed-data
[setup the test environment]: https://github.com/rails/rails/pull/47719/files
Follow-up to #1157

Removes obsolete files and test ceremony.
Relates to #1157

For now, this generator simply creates `dev.rake` which contains
[dev:prime][].

Note that we'll want this generator to run **after**
`suspenders:factories`.

If future application specific tasks need to be added, we can use this
generator. However, this should not be confused with the existing
pattern of creating tasks under the suspenders namespace, such as the
existing `lib/tasks/suspenders.rake`. Tasks under this namespace cannot
be edited by the consumer, whereas tasks generated by `suspenders:tasks`
are intended to be edited by the consumer.

[dev:prime]: https://thoughtbot.com/blog/priming-the-pump
stevepolitodesign and others added 4 commits May 9, 2024 16:16
It's possible that `postcss.config.js` could be deleted, even if we
check for its existence.

Co-authored-by: Mike Burns <mburns@thoughtbot.com>
Since we started working on this, `3.3.1` was [released][].

[released]: https://github.com/ruby/ruby/releases/tag/v3_3_1
Generators that invoke `yarn` can result in warning being printed during
testing, such as:

```
warning " > postcss-url@10.1.3" has unmet peer dependency
"postcss@^8.0.0".
```

or

```
DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
```

Although these warnings may be legitimate, they disrupt the test output.
This prepares us to merge `suspenders-3-0-0` into `main`. Once we
formally release to RubyGems, we can remove the reference to GitHub.
@stevepolitodesign stevepolitodesign merged commit 825d94d into main May 10, 2024
2 checks passed
stevepolitodesign added a commit to thoughtbot/dotfiles that referenced this pull request May 10, 2024
Now that [Suspenders 3.0.0][] has [merged][], we can improve the
developer experience by placing all option into our `railsrc` file.

Once this commit it merged, we can link to our dotfiles from the
Suspenders README.

[Suspenders 3.0.0]: https://github.com/thoughtbot/suspenders
[merged]: thoughtbot/suspenders#1135
stevepolitodesign added a commit to thoughtbot/dotfiles that referenced this pull request May 10, 2024
Now that [Suspenders 3.0.0][] has [merged][], we can improve the
developer experience by placing all option into our `railsrc` file.

Once this commit it merged, we can link to our dotfiles from the
Suspenders README.

Also closes #746

[Suspenders 3.0.0]: https://github.com/thoughtbot/suspenders
[merged]: thoughtbot/suspenders#1135
stevepolitodesign added a commit to thoughtbot/dotfiles that referenced this pull request May 10, 2024
Now that [Suspenders 3.0.0][] has [merged][], we can improve the
developer experience by placing all option into our `railsrc` file.

Once this commit it merged, we can link to our dotfiles from the
Suspenders README.

Also closes #746

[Suspenders 3.0.0]: https://github.com/thoughtbot/suspenders
[merged]: thoughtbot/suspenders#1135
stevepolitodesign added a commit to thoughtbot/dotfiles that referenced this pull request May 10, 2024
Now that [Suspenders 3.0.0][] has [merged][], we can improve the
developer experience by placing all options into our `railsrc` file.

Once this commit it merged, we can link to our dotfiles from the
Suspenders README.

Also closes #746

[Suspenders 3.0.0]: https://github.com/thoughtbot/suspenders
[merged]: thoughtbot/suspenders#1135
stevepolitodesign added a commit that referenced this pull request May 15, 2024
With the merge of #1135 and some subsequent follow-ups, we are ready to
officially release the next version of Suspenders.
stevepolitodesign added a commit that referenced this pull request May 15, 2024
With the merge of #1135 and some subsequent follow-ups, we are ready to
officially release the next version of Suspenders.
stevepolitodesign added a commit that referenced this pull request May 16, 2024
With the merge of #1135 and some subsequent follow-ups, we are ready to
officially release the next version of Suspenders.

Because we moved to [calver][] in #1106, we need to continue using
calver, since the latest release `20230113.0` is greater than `3.0.0`,
which was set in haste in ab3eb97

In an effort to better brand this release, we give it the code name
"Tailored".

[calver]: https://calver.org
stevepolitodesign added a commit that referenced this pull request May 16, 2024
With the merge of #1135 and some subsequent follow-ups, we are ready to
officially release the next version of Suspenders.

Because we moved to [calver][] in #1106, we need to continue using
calver, since the latest release `20230113.0` is greater than `3.0.0`,
which was set in haste in ab3eb97

In an effort to better brand this release, we give it the code name
"Tailored".

[calver]: https://calver.org
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.

None yet

4 participants