Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FeatureFlag の実装 #4362

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
6 changes: 4 additions & 2 deletions src/assets/mdi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ import {
mdiCrown,
mdiFormatTitle,
mdiCloseCircle,
mdiNotebook
mdiNotebook,
mdiCodeBraces
} from '@mdi/js'

interface MdiIconsMapping {
Expand Down Expand Up @@ -167,7 +168,8 @@ const mdi: MdiIconsMapping = {
'music-note': mdiMusicNote,
stop: mdiStop,
crown: mdiCrown,
'format-title': mdiFormatTitle
'format-title': mdiFormatTitle,
'code-braces': mdiCodeBraces
}

export default mdi
52 changes: 52 additions & 0 deletions src/components/Settings/FeatureFlagTab/FeatureFlag.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<template>
<div :class="$style.container">
<div :class="$style.description">
<h3>{{ props.title }}</h3>
<p>{{ props.description }}</p>
<h4 v-if="FlagStatus['flag_test']">ていきょうしゅうりょうび</h4>
<h4 v-else>提供終了日</h4>
<p>
{{ props.endAt.toLocaleDateString() }}
</p>
Comment on lines +6 to +10
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

細かいですが、提供終了日は横に並べた方が見やすいかもしれません

提供終了日
2024/08/31

ではなく

提供終了日 2024/08/31

みたいにするということです

</div>
<div>
<a-toggle v-model="value" :disabled="props.endAt < new Date()" />
</div>
</div>
</template>

<script lang="ts" setup>
import AToggle from '/@/components/UI/AToggle.vue'
import { useModelValueSyncer } from '/@/composables/useModelSyncer'
import { useFeatureFlagSettings } from '/@/store/app/featureFlagSettings'

const { FlagStatus } = useFeatureFlagSettings()

const props = defineProps<{
title: string
description: string
modelValue: boolean
endAt: Date
}>()

const emit = defineEmits<{
(e: 'update:modelValue', _val: boolean): void
}>()

const value = useModelValueSyncer(props, emit)
</script>

<style lang="scss" module>
.container {
display: flex;
align-items: center;
justify-content: space-between;
gap: 1.5rem;
}

.description {
display: flex;
flex-direction: column;
gap: 0.25rem;
}
</style>
8 changes: 7 additions & 1 deletion src/components/Settings/composables/useNavigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ export const navigationRouteNameTitleMap: Record<SettingsRouteName, string> = {
settingsBrowser: 'ブラウザ',
settingsQall: '通話 (Qall)',
settingsStamp: 'スタンプ',
settingsTheme: 'テーマ'
settingsTheme: 'テーマ',
settingsFeatureFlag: '実験的機能'
}

export const navigations: {
Expand Down Expand Up @@ -60,6 +61,11 @@ export const navigations: {
routeName: 'settingsTheme',
iconName: 'brightness-6',
iconMdi: true
},
{
routeName: 'settingsFeatureFlag',
iconName: 'code-braces',
iconMdi: true
}
]

Expand Down
7 changes: 6 additions & 1 deletion src/router/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export type SettingsRouteName =
| 'settingsQall'
| 'settingsStamp'
| 'settingsTheme'
| 'settingsFeatureFlag'

export const isSettingsRouteName = (
name: string
Expand All @@ -33,6 +34,8 @@ const pathByRouteName = (routeName: SettingsRouteName) => {
return 'stamp'
case 'settingsTheme':
return 'theme'
case 'settingsFeatureFlag':
return 'feature-flag'
}
}

Expand All @@ -42,6 +45,7 @@ const Browser = () => import('/@/views/Settings/BrowserTab.vue')
const Qall = () => import('/@/views/Settings/QallTab.vue')
const Stamp = () => import('/@/views/Settings/StampTab.vue')
const Theme = () => import('/@/views/Settings/ThemeTab.vue')
const FeatureFlag = () => import('/@/views/Settings/FeatureFlagTab.vue')

const createRoute = (name: SettingsRouteName, component: Component) => ({
name,
Expand All @@ -55,7 +59,8 @@ export const settingsRoutes: RouteRecordRaw[] = [
createRoute('settingsBrowser', Browser),
createRoute('settingsQall', Qall),
createRoute('settingsStamp', Stamp),
createRoute('settingsTheme', Theme)
createRoute('settingsTheme', Theme),
createRoute('settingsFeatureFlag', FeatureFlag)
]

export const defaultSettingsName: SettingsRouteName = 'settingsProfile'
Expand Down
118 changes: 118 additions & 0 deletions src/store/app/featureFlagSettings.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
import { defineStore, acceptHMRUpdate } from 'pinia'
import { computed } from 'vue'
import { convertToRefsStore } from '/@/store/utils/convertToRefsStore'
import useIndexedDbValue from '/@/composables/utils/useIndexedDbValue'
import { getVuexData } from '/@/store/utils/migrateFromVuex'
import { isObjectAndHasKey } from '/@/lib/basic/object'
import { promisifyRequest } from 'idb-keyval'

type FeatureFlagKey = 'flag_test'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

やらなくてもいいのですが、featureFlagDescriptions を as const satisfies ... にして、そこから型をこねる (keyof typeof featureFlagDescriptions) と FeatureFlagKey という型は自動で導出できそうです


type State = {
status: Map<FeatureFlagKey, boolean | undefined>
}

type FeatureFlagDescription = {
title: string
description: string
defaultValue: boolean
endAt: Date // 最大一ヶ月先を指定し、それ以上先は指定しない
}

export type FeatureFlag = FeatureFlagDescription & { enabled: boolean }

/*
*** FeatureFlag の利用仮ルール ***
- 日時の指定は最大一ヶ月先まで (FeatureFlag の利用を前提とした・FeatureFlag を乱用した運用を避けるため。期間後に本実装するかを検討する)
- 利用しなくなった FeatureFlag は削除する
- 仮ルールなので必要に応じて変えてほしいです
*/

export const featureFlagDescriptions: Record<
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ここをMapにしてしまえばFeatureFlagsなどの内部でasを使わないで済みそうです。Recordにする特別な理由がないならMapでいいのかなーと思います。

FeatureFlagKey,
FeatureFlagDescription
> = {
flag_test: {
title: 'フラグテスト・サンプル用',
description: '「提供終了日」の表記がひらがなになります。',
defaultValue: true,
endAt: new Date('2024-08-31')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

これだとUTCになっちゃいそうですね (まぁ問題ないと言えばないが)

}
}

const useFlagSettingsPinia = defineStore('app/featureFlagSettings', () => {
const initialValue: State = {
status: new Map<FeatureFlagKey, boolean | undefined>()
}

const [state, restoring, restoringPromise] = useIndexedDbValue(
'store/app/featureFlagSettings',
1,
{
// migrate from vuex
1: async getStore => {
const vuexStore = await getVuexData()
if (!vuexStore) return
if (!isObjectAndHasKey(vuexStore, 'app')) return
if (!isObjectAndHasKey(vuexStore.app, 'featureFlagSettings')) return
const addReq = getStore().add(vuexStore.app.featureFlagSettings, 'key')
await promisifyRequest(addReq)
}
},
initialValue
)

const isFlagEnabled = (flag: FeatureFlagKey): boolean => {
const featureFlag = featureFlagDescriptions[flag]
if (featureFlag.endAt < new Date()) {
return false
}
return state.status.get(flag) ?? featureFlag.defaultValue
}

const FeatureFlags = computed(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

この関数はfeatureFlagDescriptions以外に依存する変数が無さそうに見えて、computedにする必要ないのではと思ったのですが、何か理由があったりしますか?

const res: Map<FeatureFlagKey, FeatureFlag> = new Map()
Object.entries(featureFlagDescriptions).forEach(([flag, featureFlag]) => {
res.set(flag as FeatureFlagKey, {
title: featureFlag.title,
description: featureFlag.description,
defaultValue: featureFlag.defaultValue,
endAt: featureFlag.endAt,
enabled: isFlagEnabled(flag as FeatureFlagKey)
})
})
return res
})

const FlagStatus = computed(() => {
const res: Record<FeatureFlagKey, boolean> = {} as Record<
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FeatureFlagsがMapでこっちがRecordなのって何か理由ありますか?ないなら統一しちゃいたいです。

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

使う時はRecordの方が使いやすいと思うので、Recordに統一するのが良さそうに見えます

FeatureFlagKey,
boolean
>
Object.entries(featureFlagDescriptions).forEach(([flag, featureFlag]) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

細かいですけど、featureFlagは使われてないので無くても良さそうです

res[flag as FeatureFlagKey] = isFlagEnabled(flag as FeatureFlagKey)
})
return res
})

const updateFeatureFlagStatus = async (
flag: FeatureFlagKey,
enabled: boolean
) => {
await restoringPromise.value
state.status.set(flag, enabled)
}

return {
updateFeatureFlagStatus,
FeatureFlags,
FlagStatus,
restoring
}
})

export const useFeatureFlagSettings = convertToRefsStore(useFlagSettingsPinia)

if (import.meta.hot) {
import.meta.hot.accept(acceptHMRUpdate(useFlagSettingsPinia, import.meta.hot))
}
34 changes: 34 additions & 0 deletions src/views/Settings/FeatureFlagTab.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<template>
<section :class="$style.featureFlagTab">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

featureFlagTab のスタイルが効いてない (間隔を開けたいんだと思うが、実際に子要素はdiv1つしかないので意味がない) ので、上手くやると良さそうです

<div v-if="!restoring">
<feature-flag
v-for="[key, item] in FeatureFlags"
:key="key"
:title="item.title"
:description="item.description"
:model-value="FlagStatus[key]"
:end-at="item.endAt"
@update:model-value="(v: boolean) => updateFeatureFlagStatus(key, v)"
/>
</div>
<div v-else>
<p>Now loading...</p>
</div>
</section>
</template>

<script lang="ts" setup>
import { useFeatureFlagSettings } from '/@/store/app/featureFlagSettings'
import FeatureFlag from '/@/components/Settings/FeatureFlagTab/FeatureFlag.vue'

const { updateFeatureFlagStatus, FeatureFlags, FlagStatus, restoring } =
useFeatureFlagSettings()
</script>

<style lang="scss" module>
.featureFlagTab {
display: flex;
flex-direction: column;
gap: 1.5rem;
}
</style>
Loading