LoginSignup
0
0

More than 1 year has passed since last update.

【JS編】未経験〜これまでに受けた「コードレビュー」まとめ

Last updated at Posted at 2021-10-24

はじめに

対象読者

  • JavaScriptをある程度自分で勉強された方(簡単なアプリなど作ったことのある方)
  • これから実務に入るにあたって技術面で意識すべきことが知りたい方

技術スタック

技術スタック
JS

内容について

こちらの記事では、これまでに参画した現場で受けたコードレビューや、学んだことについて記載しています。
主に技術的なこと(JS)について記載されています。

言語や、フレームワークが異なれば理解できないことや、必要な知識としてマッチしないものがあるかと思います。
しかし、多少なりとも参考になることがあるはずです。

「重要度」の高いと思うものから記載していますので、
現場で求められているコードの書き方などが少しでも伝わればと思います。

また、各エンジニアによってレビューの内容は異なります。
同じ現場のエンジニアであっても真逆の指摘を受けることは多々あります。(実際に何度もありました。)
ここに記載されていることと全く別の指摘を受けることが今後もあると思います笑

その時は、現場に合わせるなり、自分の正しいと思うものを信じるなりしてください。

技術編

⭐️⭐️やや重要

○【JS】 URLの埋め込みはしてはいけない

js側でURLを動的に変更した場合、情報漏洩に繋がる可能性があるため。

基本的に、URLはルーティング内で管理すること

○キャッシュバスティングを入れる

CSS、JS、画像パス。それぞれに対して適用させてください。

○jQueryオブジェクトを含む変数は$で始める?

これについては、賛否をうみます。

$使うべき派

  • jQueryオブジェクトだと判断しやすうように含みましょう。
sample.js
const $button = $(".button");

ちなみに、リーダブルコードにも書いていました。

$使うべきでない派

  • PHPの変数だと間違えるから使わないで

結論

現場に合わせましょう。
同じ現場で異なる指摘を受けた場合は、
自分が良いと思った使い方をした方がいいと思います。

○npmでライブラリをインストールする際の注意点

  • メンテナンスされているライブラリなこと

    • 長いあいだ更新されていないライブラリとか、ダウンロード数やGithubスター数が極端に少ないライブラリを使うのは避けましょう
    • (マイナーなライブラリを入れる時は自分がメンテナになる覚悟で)
  • メンテナンスされすぎないライブラリなこと

    • 破壊的な更新がされそうなライブラリ(特に新しいものとか)はなるべく避ける
  • 高機能すぎないこと

    • ライブラリの一部だけ使うためにでかいライブラリ入れるのはなるべく避けましょう
  • 単機能すぎないこと

    • 自分で実装できるものは自分でしましょう

⭐️参考程度に

○【ajax】レスポンスで受け取った戻り値の確認をする

done時(正常通信をした)、
サーバーが想定した戻り値を返しているかの確認をする

sample.js
.done(function(xhr) {
  if(xhr.save === 'ok') { // ここは例。適宜変更して下さい
  } else {
  }
}

○【ajax】データの送信方法にserialize()を使う

serialize()

入力された全てのElementを文字列のデータにシリアライズする。
http://semooh.jp/jquery/api/ajax/serialize/+/

複数の要素をまとめる役割があります。

指摘コード

このように、送るformのデータ分だけダラダラと記載していた。

$.ajax({
    // 省略
    data: {
        name                 : $('#registerForm input[name="name"]') .val(),
        email                : $('#registerForm input[name="email"]').val(),
        password             : $('#registerForm input[name="password"]').val(),
        password_confirmation: $('#registerForm input[name="password_confirmation"]').val(),
    },
})

修正後

スッキリした状態で同じようにデータが送れる

let $form = $(this).parents('form');

$.ajax({
    // 省略
    data: $form.serialize()
})

注意

ただ、シリアライズするとデータ量が増えてしまう場合もあるため、使うときは状況に応じて。
(1つのデータしか送らない時など、必要ないかも)

○終わりに

formでactionを使って、preventDefaultしていると矛盾が生じる。
formタグは使わずに、preventDefaultも使わずにPOSTする

今回の内容は約4ヶ月間で受けたコードレビューの内容です。
中には賛否を生むものを含まれていると思いますが、これから実務に入られる方に少しでも参考になれば幸いです。

分かりにくい箇所、質問等あればお気軽にコメントください。

別のコードレビュー記事もございますすのでご覧ください。
【Laravel編】未経験〜これまでに受けた「コードレビュー」まとめ
【技術全般】未経験〜これまでに受けた「コードレビュー」まとめ

最後まで、読んでいただきありがとうございました。

0
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
0
0