🛡️

GitHubで誤ったマージ戦略のマージを防ぐChrome拡張機能の開発をした

2023/05/11に公開

GitHub

はじめに

GitHubのPull requestのマージに際し、「Create a merge commit」「Squash and merge」「Rebase and merge」を使い分けているチームがあると思います。そんな時に、以下のような戦略を採用することはありませんか？

develop ← feature/xxxx は Squash and merge
main ← develop は Create a merge commit
develop ← main は Create a merge commit (mainにhotfixが入っている場合などを想定)

この場合、GitHubは最後に実行したマージ戦略を記憶しています。そのため、機能開発が連続する際には、何も考えずにマージボタンを押すことが可能です。ただし、機能開発が一段落した後に、main ← developのマージを行うとき、Squash and mergeが選ばれてしまう、または、develop ← feature/xxxxのマージを行うときに、Create a merge commitが選ばれてしまうという事故もあります。

実際に、GitHubのCommunityのDiscussionsで、この機能を実装してほしいという要望が寄せられていました。

現在（2023-05-11時点）、GitHub上ではこの機能を簡単に実現する方法は提供されていないようです。サードパーティー製のツールがすでに実装していないか調査したところ、似たようなものは存在しましたが、自分が求めているものとは少し異なっていました。以下に、調査結果をまとめておきます。

上記Discussionsで紹介されていたChrome拡張

Squashの選択肢そのものを消してしまう、対象はベースブランチ名のみの指定、という点で、自分が求めているものとは違いました。

意図しないSquash MergeをChrome拡張で解決する記事

この記事では、ユーザーの設定と合致していたら、マージボタンの戦略を変更するChrome拡張の作り方を紹介しています。自分が求めているものに近いですが、これは作り方のみで、Chromeの拡張機能として公開されていません。また、Forkして設定を書き換えて使う必要があり、設定もベースブランチ名に限られています。

そこで、GWの機会を利用して、自分で作ってみることにしました。最近読んだ以下の記事で、Plasmoというブラウザ拡張開発フレームワークを知り、触ってみたいと思ったことも大きいです。

作ったもの

名前は「GitHub Merge Guardian」と大層なものを付けたのですが、端的に言ってしまうと、マージボタンの色を変えるだけのものです。以下のような機能を持っています。

ユーザーは、オーナー名、リポジトリ名、ベースブランチ、比較ブランチ、マージ戦略からなる設定を複数追加可能
各項目には、ワイルドカードとして * を使用可能
設定は上から順に評価され、最初に合致したものが適用される、設定画面では並び替えも可能
現在開いているPull requestが設定に一致する場合、マージボタンの色を変更しユーザーに警告を与える
ボタンの色を変更するだけなので、設定した戦略以外のマージも可能
色は変更可能

スクリーンショット

ss1
ss2

Chrome拡張へのリンク

リポジトリ

こちらで公開しています。

開発時の工夫と躓いた点

開発時に工夫した点や躓いた点をまとめてみます。

Plasmoについて

Plasmoはブラウザ拡張の開発を支援するフレームワークです。公式サイトでは「ブラウザ拡張のためのNext.js」と謳っており、多くの機能が用意されていて開発がスムーズに進みます。
https://docs.plasmo.com/

使用してみて、特に良かったと思った機能をいくつか挙げてみます。

デフォルトでReact, TypeScriptを採用
Live-Reloading + HMRによりコード変更が即時反映されるため、開発効率が向上
Storage, Messagingなどの抽象化APIが用意されている
- 特にStorageはReact Hooksも提供されていて便利
Chromeだけでなく、クロスブラウザ対応も簡単にできる
Tailwind CSSの導入が公式でサポートされている

他にも多くの便利な機能が用意されていて、今後もブラウザ拡張を開発する際には使用しようと思います。一方で、自分にとって不足している点もいくつかあったので、それらについてもまとめてみます。

Plasmoデフォルトのtsconfig.jsonの設定が緩い

PlasmoのGet Startedで作成されるプロジェクトのtsconfig.jsonは以下のようになっています。

tsconfig.json

{
  "extends": "plasmo/templates/tsconfig.base",
  "exclude": [
    "node_modules"
  ],
  "include": [
    ".plasmo/index.d.ts",
    "./**/*.ts",
    "./**/*.tsx"
  ],
  "compilerOptions": {
    "paths": {
      "~*": [
        "./src/*"
      ]
    },
    "baseUrl": "."
  }
}

このとき、plasmo/templates/tsconfig.baseを継承しているのですが、こちらでは"strict": false, "noUnusedLocals": falseなど、設定が緩い状態になっています。スタータープロジェクトなので最初は緩い設定になっているのは理解できますが、私自身は制約が強い方が好きなので、設定を以下のように変更しました。

tsconfig.json

{
  "extends": "plasmo/templates/tsconfig.base",
  "exclude": ["node_modules"],
  "include": [".plasmo/index.d.ts", "./**/*.ts", "./**/*.tsx"],
  "compilerOptions": {
    "paths": {
      "~*": ["./src/*"]
    },
    "baseUrl": ".",

    "strict": true,
    "allowUnusedLabels": false,
    "allowUnreachableCode": false,
    "exactOptionalPropertyTypes": true,
    "noFallthroughCasesInSwitch": true,
    "noImplicitOverride": true,
    "noImplicitReturns": true,
    "noPropertyAccessFromIndexSignature": true,
    "noUncheckedIndexedAccess": true,
    "noUnusedLocals": true,
    "noUnusedParameters": true
  }
}

ESLintの導入

また、PlasmoはデフォルトではLintの類は導入されていません。個人的にはReact + TypeScriptの開発をする時に、ESLintがないと落ち着かないタイプなので、導入しました。ベースとするconfigとして、以下は鉄板だと考えており、入れています。

eslint:recommended
plugin:react/recommended
plugin:react-hooks/recommended
plugin:@typescript-eslint/recommended

また、これらに比べて影は薄い、制約は強いのですが、plugin:@typescript-eslint/recommended-requiring-type-checking の導入を個人的には勧めたいです。その理由として、@types/chromeで定義されている、ブラウザ拡張のメソッドの型定義に、any型が多く使用されていることや、Promiseの扱いのミスによる動作不良を防ぐためです。このconfigはany型に関する危ない使い方をしている時に検知してくれるno-unsafe-return, no-unsafe-call, no-unsafe-member-access などのルールや、時々ミスを誘発するno-floating-promisesやno-misused-promisesなどのルールも包含しており、堅牢な開発に役立ちます。個人の好みのルールも入っていますが、今回のプロジェクトに使用したESLintのコンフィグは以下のようになりました。

.eslintrc.cjs

module.exports = {
  env: {
    browser: true,
    es2021: true
  },
  extends: [
    "eslint:recommended",
    "plugin:react/recommended",
    "plugin:react-hooks/recommended",
    "plugin:@typescript-eslint/recommended",
    "plugin:@typescript-eslint/recommended-requiring-type-checking"
  ],
  overrides: [],
  parser: "@typescript-eslint/parser",
  parserOptions: {
    ecmaVersion: "latest",
    sourceType: "module",
    project: "./tsconfig.json"
  },
  plugins: ["react", "@typescript-eslint"],
  rules: {
    "no-console": ["error", { allow: ["warn", "error"] }],
    "react/react-in-jsx-scope": "off",
    "@typescript-eslint/no-unused-vars": ["warn", { argsIgnorePattern: "^_" }]
  },
  ignorePatterns: [".eslintrc.cjs"],
  settings: {
    react: {
      version: "detect"
    }
  }
}

Storage書き込みのDebounce

Chrome拡張開発では、データを永続的に保存するためにStorage APIを使用します。しかし、これを直接扱うのは少し面倒なので、PlasmoではこのAPIを抽象化したStorageと、それを利用したuseStorageというReact Hooksを提供しており、非常に便利です。また、本拡張機能では、マージに関するルールと色を保存するためにこの機能を使用しています。

最初は保存ボタンが押された時に、設定を保存するようにしていたのですが、ユーザビリティが悪いと判断し、ユーザーが設定を変更した時に、すぐに保存するように変更しました。以下のようなコードになります。

  const [color, setColor] = useStorage<string>(COLOR_KEY, DEFAULT_COLOR)
  // (中略)
    <input
      type="color"
      value={color}
      onChange={(e) => {
        setColor(e.currentTarget.value)
      }}
    />
  // (中略)

上記のコードは、意図通りに動作しますが、onChangeが発火するたびにStorageに書き込みが行われ、Storage APIの制限であるMAX_WRITE_OPERATIONS_PER_MINUTEに達してしまい、動作しない場面が頻発しました。このような場合、一定時間内に複数回呼ばれた場合に、最後の呼び出しのみを実行するようにするdebounceという仕組みを導入するのが一般的だと思います。

実はuseStorageでは、返り値の配列の2番目(0-indexed)に、描画用の値のみを変更する関数とストアに実際に書き込む関数を別々に提供してくれています。これを利用することで、debounceを簡単に実現できました。

function useDebouncedStorage<T>(
  key: string,
  onInit?: Setter<T>,
  timeoutMs?: number
) {
  const [value, , { setRenderValue, setStoreValue }] = useStorage<T>(
    key,
    onInit
  )

  const throttledSetStoreValue = useDebounce(setStoreValue, timeoutMs ?? DEFAULT_TIMEOUT)

  const setValue = useCallback(
    (v: T | ((prevState: T) => T)) => {
      const newValue = isFunction(v) ? v(value) : v
      setRenderValue(newValue)
      throttledSetStoreValue(newValue)
      return newValue
    },
    [setRenderValue, throttledSetStoreValue, value]
  )

  return useMemo(() => {
    return {
      value,
      setValue,
    }
  }, [value, setValue])
}

useDebounce, isFunction等の説明は端折っている、実際にはエラーハンドリング少し追加している、など実際のものと差異はありますが、雰囲気は伝わるのではないでしょうか。全容はリポジトリの方をご覧ください。注意点としては、ストアの書き込みが行われるとuseStorageの内部で、描画の値も変更されるため、debounceではなく、throttleをしたい場合は、自分で描画用の値を管理する必要があると思います。

`chrome.runtime.onMessage.addListener`の落とし穴

これはFirefoxでは動作するが、Chromeでは動作しない、というパターンの一例です。chrome.runtime.onMessage.addListenerを使って、拡張機能の他のコンポーネントから送信されたメッセージを受け取って処理をすることができます。今回の機能では、いくつかのところで使っているのですが、今回は以下の機能を例に挙げます。

Popupが開かれた時に、現在開いているPull requestに合致するルールを出す

この機能を実現するには、現在のPull requestのオーナー名、リポジトリ名、ベースブランチ、比較ブランチ名を取得する必要があります。これらの値はPopupでは直接取得することができず、ContentScript側で取得する必要があります。そのため、以下のように実装していました。なお、送信側のコードはここでは省略し、また、具体的な実装も実物とは少し異なります。

contents/github.tsx

chrome.runtime.onMessage.addListener(async (message, _, sendResponse) => {
  if (message.name === QUERY_MATCHED_SETTING) {
    // 現在のPull requestのオーナー名、リポジトリ名、ベースブランチ、比較ブランチ名を取得して、保存されているルールとマッチしたものを返す
    // 保存されているルールは、Storageに保存されているので、非同期で取ってくる必要があるので、awaitを使っている
    const setting = await getMatchedSetting()
    sendResponse(setting)
  }
})

私の環境でGoogle検索を使ってchrome.runtime.onMessageについて調査したところ、一番上に表示されるドキュメントでも、似たような実装をしているので、このような実装をしていました。しかし、この実装だと実は動作せず、settingを受け取る前に接続が切れてしまいます。このドキュメントに書かれているように、trueを返すと良い、雑に調べた結果、似たことを書かれている方が多かったので、以下のようにしましたが動作しませんでした。

contents/github.tsx

chrome.runtime.onMessage.addListener(async (message, _, sendResponse) => {
  if (message.name === QUERY_MATCHED_SETTING) {
    // 現在のPull requestのオーナー名、リポジトリ名、ベースブランチ、比較ブランチ名を取得して、保存されているルールとマッチしたものを返す
    // 保存されているルールは、Storageに保存されているので、非同期で取ってくる必要があるので、awaitを使っている
    const setting = await getMatchedSetting()
    sendResponse(setting)
    // とりあえず true を返す
    return true
  }
})

とはいえ、この問題はそもそもsendResponseに失敗していそうなので、return trueに至ってないだろうと考え、awaitしないように変更しましたが、それでも動作しませんでした。

contents/github.tsx

chrome.runtime.onMessage.addListener(async (message, _, sendResponse) => {
  if (message.name === QUERY_MATCHED_SETTING) {
    // 現在のPull requestのオーナー名、リポジトリ名、ベースブランチ、比較ブランチ名を取得して、保存されているルールとマッチしたものを返す
    getMatchedSetting().then((setting) => {
      sendResponse(setting)
    })
    
    // とりあえず true を返す
    return true
  }
})

正解はこちらです。

contents/github.tsx

chrome.runtime.onMessage.addListener((message, _, sendResponse) => {
  if (message.name === QUERY_MATCHED_SETTING) {
    // 現在のPull requestのオーナー名、リポジトリ名、ベースブランチ、比較ブランチ名を取得して、保存されているルールとマッチしたものを返す
    getMatchedSetting().then((setting) => {
      sendResponse(setting)
    })
    
    // とりあえず true を返す
    return true
  }
})

正解はasyncを外すことでした。asyncをつけていたため、trueではなくPromise<true>を返すことになっており、また、chromeは返り値がPromiseの場合をサポートしていないため、動作しませんでした。
また、上記で紹介したplugin:@typescript-eslint/recommended-requiring-type-checkingを導入しておくと、間違った実装例はno-unmisused-promisesやrequire-awaitなどのルールに引っかかるので、間違いを発見しやすくなります。

今回の問題の解決には、同様の事象に対処された記事を参考にさせていただきました。わざわざ、このセクションを書くかは迷いましたが、あまり知られていない内容かもしれないので記載しておきます。(結論はasyncを使っているからというより、trueではなく、Promise<true>を返しているから、が正確な気はしますが……)

`chrome.tabs.query`の落とし穴

今度は逆にChromeでは動作するが、Firefoxで動作しないというパターンです。
今回の機能のうち、オプションページで色を設定した時に、GitHubのPull requestを開いているページに即座に反映させるというものを例に、この落とし穴について説明します。このとき、chrome.tabs.queryを使って、現在GitHubのPull requestを開いているタブを取得し、そのタブに対してメッセージを送ることで、即座に反映させるという仕組みになっています。

background/messages/update-color.ts

const tabs = await chrome.tabs.query({url: queryUrl})
  for (const tab of tabs) {
    await sendToContentScript({
      name: UPDATE_COLOR,
      tabId: tab.id,
      body: {
        color: req.body?.color ?? DEFAULT_COLOR
      }
    })
  }

これはChromeでは動作しますが、Firefoxでは動作しませんでした。Chromeの場合は、Tabsの配列が返ってくるのですが、Firefoxでは’undefined’が返ってきます。ドキュメントを見るとFirefoxでも動作するはずなのですが、実際には動作はせず、同じ現象を報告しているものもあるため、既知の問題だと思います。

この問題を避ける場合は、以下のようにchrome.tabs.queryの第2引数のコールバックを使うものに変更します。

background/messages/update-color.ts

chrome.tabs.query({url: queryUrl}, (tabs) => {
  // 中略
})

Firefox Addon提出の審査対応(Unsafe assignment to innerHTML)

Firefox Addonとして公開するためには、まず機械的な審査が行われ、その後、人間による審査が行われます。
機械的な審査に関しては、こちらのツールで提出前に検知可能です。
https://github.com/mozilla/web-ext

実装当初、機械的な審査では以下のような警告が出ており、公開ができない状態でした。

Code: UNSAFE_VAR_ASSIGNMENT
Message: Unsafe assignment to innerHTML
File: ...

調査したところ、最終的にコードにバンドルされるReactのコードの箇所でエラーが発生しており、ざっとソースを読んだところ、setInnerHTMLという関数の中でinnerHTMLを呼んでいるようでした。
この関数はdangerouslySetInnerHTML経由で呼ばれているようで、今回のコードでは別に使っていないのですが、React本体に入っているため、コードにバンドルされているようでした。

うまくコードにバンドルされないようにできれば問題ないのですが、解決できず、今回はpnpmのpatch機能でreact-domの該当箇所を修正することで、対応しました。
将来のバージョンアップの追従が難しいので、今後何とかしたいと考えています。

改善点

ここでは、現状抱えている改善点について書いていきます。何か良い方法があれば、ぜひ教えていただければと思います。

GitHubの変更に追従する仕組みがない
- ボタンの色を変更する、などの機能はGitHubのclass名などに依存しているので、変更が起きた時に容易に壊れます
- ログインしなければ、マージボタン見えないため、E2E書くのも難しいというところで悩んでいます
バッドプラクティス感のあるコードの修正
- MutationObserver使って、ボタンの要素を取得している箇所は、何か良い方法がある気がしています
- PlasmoのAPIを使いこなせていない箇所、特にMessage周りは中途半端に乗っかっていて、逆に分かりづらい形になっているかもと思っています
- 色を変更するために、CSS Modulesで定義したCSS変数を直接変更するという方法を取っていますが、ハードコードに依存していること、そもそも他の方法がないか、という点で悩んでいます
テストの拡充
- vitestによるロジックのテストが少しだけありますが、拡張機能自体のE2Eを書けると嬉しいと思っていますが、そもそも方法から分かっていません
上述したreact-domへのパッチで回避している箇所を何とかしたい

おわりに

何か不具合や、機能追加のアイデア、改善案などがあれば、お気軽にissueを立ててください！

はじめに

作ったもの

スクリーンショット

Chrome拡張へのリンク

リポジトリ

開発時の工夫と躓いた点

Plasmoについて

Plasmoデフォルトのtsconfig.jsonの設定が緩い

ESLintの導入

Storage書き込みのDebounce

chrome.runtime.onMessage.addListenerの落とし穴

chrome.tabs.queryの落とし穴

Firefox Addon提出の審査対応(Unsafe assignment to innerHTML)

改善点

おわりに

Discussion

`chrome.runtime.onMessage.addListener`の落とし穴

`chrome.tabs.query`の落とし穴