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

💎 Rails 7.2 and YJIT #763

Closed
6 of 7 tasks
yonta opened this issue Sep 4, 2024 · 21 comments · Fixed by #772
Closed
6 of 7 tasks

💎 Rails 7.2 and YJIT #763

yonta opened this issue Sep 4, 2024 · 21 comments · Fixed by #772
Assignees

Comments

@yonta
Copy link
Collaborator

yonta commented Sep 4, 2024

Rails 7.2がきたぞ!
YJITを使ってローリスクハイリターンだ!

やること

  • Railsバージョンアップ
    • Rails 7.1.4
    • Rails 7.2.1
    • devcontainerの設定の調査
    • GitHub Actionsの設定の調査
    • rubocop-rails-omakaseの調査
    • Breakemanの調査
    • ERBLintとrubocop-erbの調査
  • YJITの有効化

参考になりそうなもの

@yonta
Copy link
Collaborator Author

yonta commented Sep 10, 2024

Ruby 3.3.5もきてるから、先にこっち上げちゃおう

@yonta
Copy link
Collaborator Author

yonta commented Sep 10, 2024

YJITはRuby3.3以上とRails7.2から

@yonta
Copy link
Collaborator Author

yonta commented Sep 10, 2024

RailsガイドのRailsアップデート手順わかりやすくて助かる。
自分でも追っかけてみて、だいたいこんな感じでいけそう。

  1. GemfileのRailsバージョンのパッチバージョンを上げる
  2. bundle update rails
  3. package.jsonのrails依存をアップデート(うちにはなし)
  4. この時点で起動、rspecを実行、警告やテスト落ちの修正
  5. rails app:updateを実行する
    • config以下のファイルを最新版Railsのデフォルトにしてくれる
    • diffを確認して、デフォルトは適用、自分たちが設定した部分は残す
  6. また起動・rspec
  7. config/initializers/new_framework_defaults_X_X.rbのコメントアウトを1行ずつ外してチェック
    • 一気に全部やってみたり、次のステップでdefautlsを書き換えて一気に試してみてもよい
    • エラー起きたら考える
  8. 上記ファイルを削除しconfig/application.rbconfig.load_defaults(X.X)を上げる
    • これでconfig/initializers/new_framework_defaults_X_X.rbはいらない
  9. Railsのマイナーバージョンを上げて繰り返す
  10. Railsのメジャーバージョンを上げて繰り返す

@yonta
Copy link
Collaborator Author

yonta commented Sep 10, 2024

Rubyのアップデートでcloudinary gemが警告出すようになった。
ostructってgemがRuby 3.5からデフォルトジェムじゃないからGemfileに追加しろ、って警告
cloudinaryのバージョンを上げることで対応。

@yonta
Copy link
Collaborator Author

yonta commented Sep 10, 2024

Rails 7.1.4に上げた。
結構デフォルト設定が変わっている。

  • 今後のアップデートをしやすく!
    • なるべくdiffは小さく
    • 設定はデフォルト優先
    • 自分たちの設定はなるべくファイル末尾におく
      • mailer設定だけは、近くに似たデフォルト設定があったから、その近くに置いた。

@yonta
Copy link
Collaborator Author

yonta commented Sep 10, 2024

Rails7.1でlisten gemがデフォルトで入らなくなってた。

  • ファイル変更を監視して自動でブラウザリロードするgem
  • ERBとかの編集には便利
  • でもJS・TSの変更はできない(はず)
    • 手動リロード、browser syncとかを使ってる

使える・使えないがあると開発時に混乱しそう。
SAKAZUKIでも削除することにする。

@yonta
Copy link
Collaborator Author

yonta commented Sep 10, 2024

デフォルト設定あれこれしたときにspring削除しちゃった…。
デフォルトRailsだとGemfile上ではコメントアウトされており、手動でオンできる。

  • spring statusで起動してるかチェックできる
  • Gemfileに追加してbundle installbundle exec spring binstub --allでインストール
  • bin/spring binstub --remove --allとGemfileから削除してbundle installでアンインストール

Railsの起動を早くするし、入れた状態を復帰しておく。

@yonta
Copy link
Collaborator Author

yonta commented Sep 10, 2024

rspecまわすとRails.application.config.secretsが7.2で使えなくなるぞってdeprecate警告でるけど、どこで使ってる?
警告の該当ライン見ても使ってないが。

とりあえず7.2にしてみてエラーが出たらまた考える。

@yonta
Copy link
Collaborator Author

yonta commented Sep 10, 2024

うおー、7.2.1にするぞ!

@yonta
Copy link
Collaborator Author

yonta commented Sep 10, 2024

7.2.1の変更点
結構かわってる

devcontainer、GitHub CI、Rubocop OMAKASE、YJIT、Brakemanあたりが気になるね。

@yonta
Copy link
Collaborator Author

yonta commented Sep 10, 2024

Rails 7.2.1をインストールしてから、rails new --database=postgresql --javascript=esbuild --devcontainer testapp_containerでどんなプロジェクトができるのか覗いてみましょうね。

@yonta
Copy link
Collaborator Author

yonta commented Sep 10, 2024

devcontainer周り

  • .devcontainerディレクトリができる
    • compose.yaml devcontainer.json Dockerfileがある
    • docker周りの設定と、VSCodeのdevcontainer使うjson設定ファイル
    • 開発向けコンテナの設定に参考になりそう
    • devcontainerという名前だし、prod環境向けdockerではないわね

ドキュメント

@yonta
Copy link
Collaborator Author

yonta commented Sep 10, 2024

GitHub CI周り

  • .github/dependabot.yml
    • dependabotの設定しかた
    • SAKAZUKIはGitHubのウェブページのSettingでやってるんだっけ?
    • ファイルでできるほうが便利そう
  • .github/workflows/ci.yml
    • CIって1つにまとめてる
      • SAKAZUKIはCheckとTestに分けてるけど、やっぱまとめるのが主流だよなぁ
      • 今さらまとめると、GitHubからのログの見た目変わるんだよなあ、やるか、うーん
    • run: bin/brakeman --no-pager
      • 「Railsの一般的なセキュリティ脆弱性がproduction環境に侵入するのを防ぐ優れた方法」 
      • もちろんGemfileにも追加されてる
      • よさそうだからSAKAZUKIにも採用したいな
    • run: bin/rubocop -f githubなるものが
      • rubocopの--formatオプションでGitHub向けの出力あるのか。真似しよう
    • run: sudo apt-get update && sudo apt-get install --no-install-recommends -y google-chrome-stable curl libjemalloc2 libvips postgresql-client
      • これってubuntu-latestにすでに入ってていらないはず?
      • GitHubのubuntu-latestランナーってくそでかイメージでなんでも入ってる印象
    • run: bin/rails db:test:prepare test test:systemあたり
      • テスト周り、なんか知らない実行方法をやってる
      • DBの設定もだいぶ簡単だし、真似したらシンプルにできるか?
      • とりあえず今のREADMEに合わせておくか
    • uses: actions/upload-artifact@v4
      • テスト落ちた時にこれでがスクリーンショットを保存してる
      • 便利!!
      • RSpecもJSのテスト失敗したときにスクリーンショット残してくれるので、これ真似して残そう

@yonta
Copy link
Collaborator Author

yonta commented Sep 10, 2024

Rubocop Omakase

TODO:もう少し設定内容を精査する必要がある

rubocop.ymlの現状

  • Cookpadのrubocop.ymlを継承
    • デフォルトで全Copsをオフ
    • 手動でいろいろなCopsをオン・設定している
  • requireで各種プラグインを読み込み
    • 既存Copsの設定・オフ
      • 例えばrubocop-rspecではMetrics/BlockLengthの除外対象にspecファイルを追加する
    • 独自Copsを生成・設定
      • 例えばrubocop-capybaraCapybara/XXXXCopsのような名前空間で生成
  • 手動で、Copsをオフ・設定
    • Cookpadやプラグインがオン・生成したCopsが対象
    • 例えば、frozen stringマジックコメントのオフ
  • 手動で、Copsをオン・設定
    • Cookpadやプラグインがオンにしていない便利なCopsが対象
    • 例えば、関数型スタイルメソッドmapではdo endより{ }を使う

現状もOmakaseも最初に全Copsをオフにしている

  • DisabledByDefaultでCopsを全部オフにしてから、使うやつだけEnabledしている
    • 現状のCookpadもそうなってた、気づかなかった
    • 後から上書きもできず、例えばStyle/AsciiCommentsなんかは常にオフになってた
    • Omakaseが指定してCopsしか使えない
      • 他のCops使うには自分で探してオンにする必要がある
      • 全部オンにして、必要ないCopsをオフにするのとどっちが楽か?
        • それってそもそもOmakase使わず自分で設定するってこと

やはり厳しく全部使っていこう!

  • TypeScriptではstrict、eslintではstrct typecheckedなど強いのを使っている
  • Rubyでも強いLinterを使おう!
  • 他rubocop.ymlの継承をやめて手動調整しよう
  • デフォルトで全部オンにして、いらないものをdisabledしていこう
    • 新しいCopsが追加されたときにとりあえずオンになってわかりやすい
    • bundle update後に自動でオンになってから、不要ならオフにしよう

デフォルトDisabledだけど有用そうなCops

Layout/FirstXXXBreak:

  • Layout/FirstArrayElementLineBreakとか
  • 複数リテラルの括弧開きの後の改行を強制する
  • いれても良いかなぁ、いれなくても良いかなぁ
  • よく悩むしいれておこう
# bad
func(a,
     b)
# good
func(
  a,
  b,
)

Layout/MultilineAssignmentLayout

  • 代入イコールの後に改行をいれるかどうか
  • よく悩むしいれておこう
# bad
foo = if expression
  'bar'
end

# good
foo =
  if expression
    'bar'
  end

Layout/RedundantLineBreak

  • なるべく1行に収める
  • fmt便利なんでいれよう

Layout/SingleLineBlockChain

  • メソッドチェーンでブロック引数があるときは改行をいれる
  • 見やすいしいれよう

Style/ArrayCoercion

  • [arg]よりArray(arg)の形を優先する
  • 思わぬ[nil]のようなにるにるを防げるし入れておくべき

Style/ArrayFirstLast

  • arr[0]よりarr.firstを優先
  • いれよう

Style/AutoResourceCleanup

  • f = File.open("file")よりFile.open("file") do |f| ~~~ endを優先
  • 前者は手動でf.closeしないといけない
  • 後者は自動でファイルクローズしてくれる
  • 入れたほうがいい

Style/CollectionMethods

  • collectmapのように別名を持つメソッドで片方を強制する
  • いれよう

Style/ReturnNil

  • retun nil でなくreturnに統一

Copsの調整

現状維持

  • コメントをスペースでレイアウトするのを許す
  • 日本語コメントを許可
  • eachなどの手続きスタイルはdo ~ endmapなどの関数型スタイルは{ ~ }を使う
  • ドキュメント無しクラスを許可
  • メソッド呼び出しは基本括弧をつける、ただしマクロやRSpec DSLは括弧無しを許可
  • 文字列はダブルクオートを使う
  • 複数リテラルの末尾カンマを許可

現状のRSpecでのcontextで書ける文章のCops

  • RSpec/ContextWordingで設定
  • 現状は下記のように多く使えるようにしてあった
  • デフォルトはwhen/with/withoutのみ。
    • それでもいけるような気がするから残すか悩む
RSpec/ContextWording:
  Prefixes:
    - when
    - with
    - without
    - if
    - unless
    - for
    - before
    - after
    - during

rubocop-performance

  • rubocop-rails-omakaseをいれると一緒に入るプラグイン
  • 便利そうだからこれは採用しよう
  • autocorrectオプションで自動でパフォーマンス改善してくれて便利~!

@yonta
Copy link
Collaborator Author

yonta commented Sep 10, 2024

YJIT

  • Ruby 3.3以上ならデフォルトでオンになる
  • オフにするためにはRails.application.config.yjit = falseを設定する
  • デフォルトオン、ヨシ!

@yonta
Copy link
Collaborator Author

yonta commented Sep 10, 2024

Brakeman

Brakeman は、Ruby on Rails アプリケーションのセキュリティ脆弱性をチェックする静的解析ツールです。

  • よさそう、入れよう。
  • テキトーに実行してみたけど、SAKAZUKIは0エラー0警告
  • ロゴがルビーが入ったランタンでかわいい

設定とか

  • --run-all-checksをつけると使われるチェッカーが増える
    • とりあえずデフォルトこれでやってみる
    • 現状のSAKAZUKIではエラー0
  • --format github
    • デフォルトはtext
    • Actionsでgithub指定で使う
    • 他にもHTMLとか豊富にある
  • マーケットプレイスにActionsもいっぱいある
    • 公式ではない?
    • reviewdogとの連携や、code review形式コメントをつけたり、便利なActionsがある
    • dev環境で使うためどうせGemfileでインストールするし、とりあえずCLI呼び出しを使う

@yonta
Copy link
Collaborator Author

yonta commented Sep 11, 2024

ひょっとしてSpringももういらないか。
Rails公式も「今のマシンスペックだと対して効果ないからデフォルトオフにするわ」って言ってるな。

実際、spring周りで複雑になって起動しないとかトラブルのも辛い。
消すかー。

rails/rails#42997

@yonta
Copy link
Collaborator Author

yonta commented Sep 13, 2024

bundle updateかけて全部アップデートすると、webdirverがアップデートされてcapybaraがwebdriverのdeprecated警告だすようになっちゃうな。
Rails 7.2を動かすためのアップデート以外のbundle updateは後回しにしよう。

@yonta
Copy link
Collaborator Author

yonta commented Sep 13, 2024

Rails 7.2してrails app:update!!!

  • 大きな変更はない。
  • コメントの誤字修正や、一部デフォルト設定の変更のようだ。
  • 非対応ブラウザに対するエラーページが追加された
  • icon.pngとicon.svgが付属するようになったが、すでにあるので削除
    • 画像の内容はRuby色の■でした

@yonta yonta mentioned this issue Sep 13, 2024
@yonta
Copy link
Collaborator Author

yonta commented Sep 19, 2024

Rubocopの設定変えたら、ERBファイルがめっちゃLintに引っ掛かるようになった。
そりゃそうだ。
直すぞ!

ERBファイルのLinter

何やら新しいの見つけたので、比較してみる。

erb_lint

  • 現状使っている
  • Shopfyが積極的にメンテし続けている
  • .erb-lint.ymlで設定が可能
  • Rubocop以外にもerblintが定義している各種Linterがありリッチ
    • 例えば、<%の後にスペースが入っているか、とか
  • 1つの<% %>ルビーブロックごとに解析するので、複数ブロックにまたがる解析はできない・エラーする
    • 例えば、<% a = 1 %>の後にaを使う箇所があっても、unuseに引っ掛かる
    • <% if n == 0 %>は未完成のif式なので解析できない
      • Style/NumericPredicateによるn.zero?を使え、という解析ができない(致命的)

rubocop-erb

  • ERBのLintをrubocopに統合したプラグイン
  • 個人っぽいけど、最近も更新を続けている
  • 独自設定はなく、.rubocop.ymlの設定がそのまま使われる
  • <%のようなインデントがあると、インデントレベルを認識できない(致命的)
    • 多数のLayout/RedundantLineBreakで引っ掛かる、実際はredundantではない
  • <% page_title "Test" %>を検知できない(致命的)
    • 前後の空白が多いが、難しくて解析できないらしい(Issueにあって、PRをあきらめている)
    • 括弧なし呼び出しだがこれも検出できない

VS

erblint rubocop-erb
エラー数 28 48 複数ブロックにまたがって解析するrubocop-erbのほうが検出数が多い
autocorrect 28 47 エラー数に比例して多いだけなので差はあまりない
.rubocop.ymlを継承 o o
.rubocop.ymlを継承して一部変更 o できるが、設定を変えるには別ファイルが増える。例えば、.rubocop-erb.ymlを作って以下のようにして、rubocop --config .rubocop-erb.ymlと叩く必要がある
<% if n == 0 %> x o 未完成の式なので、Style/NumericPredicateが検出できない
<%a %> o x erblintは独自Linterで検出
inherit_mode:
  merge:
    - Exclude

inherit_from: .rubocop.yml

require:
  - rubocop-erb

AllCops:
  Include:
    - "**/*.html.erb"

Style/MethodCallWithArgsParentheses:
  IgnoreMacros: false

チェック用ERBファイル

<%    page_title "Test"    %>
<%= t "this.is.also.good" %>
<% n = 0 %>
<% b = if n == 0 then 1 else 2 end %>

<%
  def hogehogehogehogehogehoge(str1, str2)
    str1 + str2
  end
%>
<% if n == 0 %>
  <% a = 1 %>
<% else %>
  <% a = hogehogehogehogehogehoge "012345678901234567890123456789012345678901234567890123456789",
         "abcdefghijklmnopq" %>
<% end %>
<html>
  <body>
    <%= a %>
  </body>
</html>

できることとできないことあるし、併用するか…。

@yonta yonta self-assigned this Sep 21, 2024
@yonta
Copy link
Collaborator Author

yonta commented Sep 21, 2024

でかい変更になったので、baseブランチを作ってそこに各種変更を1個ずつマージPRにする方式にしよう

@yonta yonta mentioned this issue Oct 30, 2024
@yonta yonta reopened this Nov 26, 2024
@yonta yonta mentioned this issue Jan 7, 2025
@yonta yonta closed this as completed Jan 7, 2025
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 a pull request may close this issue.

1 participant