生涯未熟

生涯未熟

プログラミングをちょこちょこと。

Boostnoteにcontributeしてみた

初めてOSSにcontributeしてみました。

経緯としてはこんな感じ。

  • ん?Boostnoteでノートが削除できないパターンがある
  • そういえばBoostnoteってOSSだったか
  • それじゃcontributeしてみようか!

ただ、Reduxでガッチリ作り込まれてたので修正に深夜1時から朝の6時までかかりました・・・

修正内容自体はそんなになのにねー、他人のコード読むの難しいね。

修正内容

まずは追いかける

今回の問題としては「最後尾のノートが削除できない」というものでした。

コード全体を見るのも骨が折れるので、まずは削除ボタンの挙動から追っかけてみました。

f:id:syossan:20160731031234p:plain

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>

Boostnote/MarkdownNoteDetail.js at 49acd8a4f3fd885b5fac81eba2b5546ae9fee708 · BoostIO/Boostnote · GitHub

ふむふむ、 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')
      })
  }

Boostnote/MarkdownNoteDetail.js at 49acd8a4f3fd885b5fac81eba2b5546ae9fee708 · BoostIO/Boostnote · GitHub

ぽい処理が見つかりましたね!

この処理を見るに 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していただけました!

f:id:syossan:20160731034255p:plain

こうして、初めてのOSSへのcontribute体験はなんとか良い結果になりました。

contributeとかめっちゃ難しいもんなんじゃね?とか勝手に思ってましたが、案外こんな弱小エンジニアでもcontribute出来るもんだなーということが分かっただけでも儲けものでした。

これからも気が向いた時にOSSへ貢献していくぞー。