APIクライアントのFactoryを作って、動的にAPIクライアントを変更できるようにする #420
Conversation
src/store/audio.ts
Outdated
| @@ -112,6 +109,7 @@ export const audioStoreState: AudioStoreState = { | |||
| audioKeys: [], | |||
| audioStates: {}, | |||
| nowPlayingContinuously: false, | |||
| _engineFactory: OpenAPIEngineConnectorFactory, | |||
There was a problem hiding this comment.
AudioStore に持たせるのもなんか違う気がしている
window.electronとか使ってるのもinfrastructuresとか言う層として扱って、別途Storeを作り、そのStateに突っ込むのがいいのではないかとか思っている
There was a problem hiding this comment.
たしかに別Storeがあっても良いかなと思いました。
vuexの仕様がわからないのですが、OpenAPIEngineConnectorFactoryをstateに入力しても良いのかがわからないでいます。
Vueのrefやreactiveに入力可能なものであれば良いのかなと思いました。
（ref(関数)ってできるんでしょうか・・・？）
There was a problem hiding this comment.
FirebaseのUserをStateに突っ込んでいるプロダクトとかも調べてみるとあるので、大丈夫なのかなとは思っていましたが、Documentをちゃんと読んでいないので断言できないですね…
またIssueにStateはShould be serializableだとは言っているので、今回のはあまり良くなさそうではありますね…
vuejs/vuex#757 (comment)
（2017年から変わったのかしら…）
There was a problem hiding this comment.
こんな感じでStoreCreatorを作って、依存を流し込んだものを外部にexportするという形にしてみましたがどうでしょうか…
https://github.com/Hiroshiba/voicevox/pull/420/files#diff-b86b438c7928773aac473007988ffb602afc8278b8e2226ce784442befb46b0dR117-R124
https://github.com/Hiroshiba/voicevox/pull/420/files#diff-b86b438c7928773aac473007988ffb602afc8278b8e2226ce784442befb46b0dR918
There was a problem hiding this comment.
AudioStoreのtestをしたくなったらCreatorもexportして、IEngineConnectorFactoryを実装したものを流し込めば良いのかな、と思ってます
There was a problem hiding this comment.
良いと思います！！
ネストが一段増えちゃうのでちょっとだけコーディングが難しくなったかもですが、まあやろうと思えばmutationとかだけ取り出せたりもするので、この形で良いのかなと思いました。
src/store/audio.ts
Outdated
| @@ -112,6 +109,7 @@ export const audioStoreState: AudioStoreState = { | |||
| audioKeys: [], | |||
| audioStates: {}, | |||
| nowPlayingContinuously: false, | |||
| _engineFactory: OpenAPIEngineConnectorFactory, | |||
There was a problem hiding this comment.
たしかに別Storeがあっても良いかなと思いました。
vuexの仕様がわからないのですが、OpenAPIEngineConnectorFactoryをstateに入力しても良いのかがわからないでいます。
Vueのrefやreactiveに入力可能なものであれば良いのかなと思いました。
（ref(関数)ってできるんでしょうか・・・？）
|
うーん、このPR、Factoryから引っ張ってくるのと、依存を外部に移すのが混じってしまってるな… |
yamachu
force-pushed
the
api-interface
branch
from
d3c098f
to
fe50fd2
Compare
yamachu
force-pushed
the
api-interface
branch
from
fe50fd2
to
23d3417
Compare
|
コンフリクトしていそうなので解消お願いします。 |
|
Hiroshiba@1be4349 |
内容
OpenAPIのAPIクライアントは audio.ts でインスタンス化され、Storeで対象のインスタンスに依存するように実装されている。
そのため動的にインスタンスを変更するのが困難であり、StoreのMutationなどにEngineに接続するためのClientの変更を行う関数などを実装をするなどの変更が必要である。
また audio.ts は音声に関する知識だけを持っていてほしいので設計がいびつになることは想像に難くない。
そのため本PRではstoreに直接APIクライアントに依存させず、storeの状態に管理されたFactoryに依存する形に変更する。
この変更により、FactoryのInterfaceを実装したMockのFactoryをStoreのstateに入れることで、EngineなしにUIやStoreの開発も容易になる。（実際の所electron依存のところもInterfaceにする必要はある…）
関連 Issue
ref: https://github.com/Hiroshiba/voicevox/issues/220
スクリーンショット・動画など
その他