初めてOSSにcontributeしてみました。
経緯としてはこんな感じ。
- ん?Boostnoteでノートが削除できないパターンがある
- そういえばBoostnoteってOSSだったか
- それじゃcontributeしてみようか!
ただ、Reduxでガッチリ作り込まれてたので修正に深夜1時から朝の6時までかかりました・・・
修正内容自体はそんなになのにねー、他人のコード読むの難しいね。
修正内容
まずは追いかける
今回の問題としては「最後尾のノートが削除できない」というものでした。
コード全体を見るのも骨が折れるので、まずは削除ボタンの挙動から追っかけてみました。
confirm
ボタンのelementが以下になります。
<button class="MarkdownNoteDetail__info-delete-confirmButton___browser-main-Detail-">Confirm</button>
ここから info-delete-confirmButton
を追っかけてみると、以下のコードを発見。
<button styleName='info-delete-confirmButton' onClick={(e) => this.handleDeleteConfirmButtonClick(e)} ref='deleteConfirmButton' >Confirm</button>
ふむふむ、 handleDeleteConfirmButtonClick
を実行してるっぽいので更に追いかけてみましょう。
handleDeleteConfirmButtonClick (e) { let { note, dispatch } = this.props dataApi .removeNote(note.storage, note.folder, note.key) .then(() => { let dispatchHandler = () => { dispatch({ type: 'REMOVE_NOTE', note: note }) } ee.once('list:moved', dispatchHandler) ee.emit('list:next') ee.emit('list:focus') }) }
ぽい処理が見つかりましたね!
この処理を見るに REMOVE_NOTE
のdispatchを list:moved
に紐付けていて、次に list:moved
がemitされた時にノートが削除される感じですね。
という訳で、次に list:moved
がemitされる処理を探しました。
selectNextNote () { if (this.notes == null || this.notes.length === 0) { return } let { router } = this.context let { location } = this.props let targetIndex = _.findIndex(this.notes, (note) => { return note.uniqueKey === location.query.key }) if (targetIndex === this.notes.length - 1) { return } targetIndex++ if (targetIndex < 0) targetIndex = 0 else if (targetIndex > this.notes.length - 1) targetIndex === this.notes.length - 1 router.push({ pathname: location.pathname, query: { key: this.notes[targetIndex].uniqueKey } }) ee.emit('list:moved') }
Boostnote/index.js at d73b567bd466d608b35a57527a84355e61054d66 · BoostIO/Boostnote · GitHub
お!ありましたね!
list:next
を追いかけたら発見しました。
で、今度はこのコードをじっくり読み解いていくことに・・・
コードを読み解く
Developer Toolをフル活用して読み解いていくと、最後尾のノートを削除した時に以下のコードが実行されていました。
if (targetIndex === this.notes.length - 1) { return }
で、ここで気付いたわけです。
「おや?最後尾のノートの削除時にここでreturnされるってことは list:moved
がemitされないじゃないか・・・」と。
なので、ここをゴニョゴニョして削除されるように直しました。
修正した結果は以下のとおりです。
selectNextNote () { if (this.notes == null || this.notes.length === 0) { return } let { router } = this.context let { location } = this.props let targetIndex = _.findIndex(this.notes, (note) => { return note.uniqueKey === location.query.key }) if (targetIndex === this.notes.length - 1) { targetIndex = 0 } else { targetIndex++ if (targetIndex < 0) targetIndex = 0 else if (targetIndex > this.notes.length - 1) targetIndex === this.notes.length - 1 } router.push({ pathname: location.pathname, query: { key: this.notes[targetIndex].uniqueKey } }) ee.emit('list:moved') }
Boostnote/index.js at e256c7a7d939790ccb9e9efd2a758f645b28d3ff · BoostIO/Boostnote · GitHub
returnを排して、targetIndex(削除後の移動先)を0に固定した感じですね。
なんとかこれで動くことを確認したので、PRを送ったところ作者の方にmergeしていただけました!
こうして、初めてのOSSへのcontribute体験はなんとか良い結果になりました。
contributeとかめっちゃ難しいもんなんじゃね?とか勝手に思ってましたが、案外こんな弱小エンジニアでもcontribute出来るもんだなーということが分かっただけでも儲けものでした。
これからも気が向いた時にOSSへ貢献していくぞー。