FeatureFlag の実装 #4362
FeatureFlag の実装 #4362
Conversation
|
Preview (prod) → https://4362-prod.traq-preview.trapti.tech/ |
Takeno-hito
force-pushed
the
feat/feature-flag
branch
from
0fb58fe
to
b8bdb7c
Compare
Takeno-hito
force-pushed
the
feat/feature-flag
branch
from
b8bdb7c
to
a53baf1
Compare
There was a problem hiding this comment.
実装は基本は問題無さそうですがいくつか気になった部分をコメントしました!
運用面で気になったポイントとしては以下のような感じです
- 本実装する際に、FeatureFlagと同じ動作をすることをどうやって保証するか。現状切り替えの際に違う動作になることがあったりしそうだから、「FeatureFlag部分は別のコンポーネントに分ける」みたいなルールを決めたい。
- FeatureFlagの実装の告知をどこでやるのか。全体通知はやりすぎな気がする。
- リリースサイクルをどうするか。この機会にマージされる事にリリースするようにしてもいいのかもしれないけど議論がややこしくなりそう。
ここらへんがめんどくさそうだからPTBみたいなのもありかなーとはちょっと思ったけど、それはそれで画像とか引用とかまわりが難しそうだからせっかく作ってもらったし、FeatureFlagで一旦運用してみるのでいいのかなと思ってます
| return state.status.get(flag) ?? featureFlag.defaultValue | ||
| } | ||
|
|
||
| const FeatureFlags = computed(() => { |
There was a problem hiding this comment.
この関数はfeatureFlagDescriptions以外に依存する変数が無さそうに見えて、computedにする必要ないのではと思ったのですが、何か理由があったりしますか?
| FeatureFlagKey, | ||
| boolean | ||
| > | ||
| Object.entries(featureFlagDescriptions).forEach(([flag, featureFlag]) => { |
There was a problem hiding this comment.
細かいですけど、featureFlagは使われてないので無くても良さそうです
| - 仮ルールなので必要に応じて変えてほしいです | ||
| */ | ||
|
|
||
| export const featureFlagDescriptions: Record< |
There was a problem hiding this comment.
ここをMapにしてしまえばFeatureFlagsなどの内部でasを使わないで済みそうです。Recordにする特別な理由がないならMapでいいのかなーと思います。
| }) | ||
|
|
||
| const FlagStatus = computed(() => { | ||
| const res: Record<FeatureFlagKey, boolean> = {} as Record< |
There was a problem hiding this comment.
FeatureFlagsがMapでこっちがRecordなのって何か理由ありますか?ないなら統一しちゃいたいです。
There was a problem hiding this comment.
使う時はRecordの方が使いやすいと思うので、Recordに統一するのが良さそうに見えます
| title: 'フラグテスト・サンプル用', | ||
| description: '「提供終了日」の表記がひらがなになります。', | ||
| defaultValue: true, | ||
| endAt: new Date('2024-08-31') |
There was a problem hiding this comment.
これだとUTCになっちゃいそうですね (まぁ問題ないと言えばないが)
| @@ -0,0 +1,34 @@ | |||
| <template> | |||
| <section :class="$style.featureFlagTab"> | |||
There was a problem hiding this comment.
featureFlagTab のスタイルが効いてない (間隔を開けたいんだと思うが、実際に子要素はdiv1つしかないので意味がない) ので、上手くやると良さそうです
| <h4 v-if="FlagStatus['flag_test']">ていきょうしゅうりょうび</h4> | ||
| <h4 v-else>提供終了日</h4> | ||
| <p> | ||
| {{ props.endAt.toLocaleDateString() }} | ||
| </p> |
There was a problem hiding this comment.
細かいですが、提供終了日は横に並べた方が見やすいかもしれません
提供終了日
2024/08/31
ではなく
提供終了日 2024/08/31
みたいにするということです
| import { isObjectAndHasKey } from '/@/lib/basic/object' | ||
| import { promisifyRequest } from 'idb-keyval' | ||
|
|
||
| type FeatureFlagKey = 'flag_test' |
There was a problem hiding this comment.
やらなくてもいいのですが、featureFlagDescriptions を as const satisfies ... にして、そこから型をこねる (keyof typeof featureFlagDescriptions) と FeatureFlagKey という型は自動で導出できそうです
|
考慮したいことが多いのと現状あまりメリットを感じられなくなったので一旦draftにします |
fix: #4357