Respond to PR comments.
This commit is contained in:
parent
0eaf6b7d42
commit
c14f108d4d
|
@ -83,22 +83,18 @@ class NumberFlag extends Flag<number> {
|
||||||
}
|
}
|
||||||
|
|
||||||
export class FlagSet<V extends Flag<any>, T extends { [key: string]: V }> {
|
export class FlagSet<V extends Flag<any>, T extends { [key: string]: V }> {
|
||||||
private readonly flags: T
|
constructor(private readonly flagSchema: T) {}
|
||||||
|
|
||||||
constructor(flags: T) {
|
|
||||||
this.flags = flags
|
|
||||||
}
|
|
||||||
|
|
||||||
defaults(): FlagValues<T> {
|
defaults(): FlagValues<T> {
|
||||||
return Object.keys(this.flags).reduce((acc, key) => {
|
return Object.keys(this.flagSchema).reduce((acc, key) => {
|
||||||
const typedKey = key as keyof T
|
const typedKey = key as keyof T
|
||||||
acc[typedKey] = this.flags[key].defaultValue
|
acc[typedKey] = this.flagSchema[key].defaultValue
|
||||||
return acc
|
return acc
|
||||||
}, {} as FlagValues<T>)
|
}, {} as FlagValues<T>)
|
||||||
}
|
}
|
||||||
|
|
||||||
isFlagName(name: string | number | symbol): name is keyof T {
|
isFlagName(name: string | number | symbol): name is keyof T {
|
||||||
return this.flags[name as keyof T] !== undefined
|
return this.flagSchema[name as keyof T] !== undefined
|
||||||
}
|
}
|
||||||
|
|
||||||
async get<K extends keyof T>(
|
async get<K extends keyof T>(
|
||||||
|
@ -120,9 +116,7 @@ export class FlagSet<V extends Flag<any>, T extends { [key: string]: V }> {
|
||||||
async fetch(ctx?: UserCtx): Promise<FlagValues<T>> {
|
async fetch(ctx?: UserCtx): Promise<FlagValues<T>> {
|
||||||
return await tracer.trace("features.fetch", async span => {
|
return await tracer.trace("features.fetch", async span => {
|
||||||
const tags: Record<string, any> = {}
|
const tags: Record<string, any> = {}
|
||||||
|
const flagValues = this.defaults()
|
||||||
const flags = this.defaults()
|
|
||||||
|
|
||||||
const currentTenantId = context.getTenantId()
|
const currentTenantId = context.getTenantId()
|
||||||
const specificallySetFalse = new Set<string>()
|
const specificallySetFalse = new Set<string>()
|
||||||
|
|
||||||
|
@ -146,12 +140,13 @@ export class FlagSet<V extends Flag<any>, T extends { [key: string]: V }> {
|
||||||
throw new Error(`Feature: ${feature} is not an allowed option`)
|
throw new Error(`Feature: ${feature} is not an allowed option`)
|
||||||
}
|
}
|
||||||
|
|
||||||
if (typeof flags[feature] !== "boolean") {
|
if (typeof flagValues[feature] !== "boolean") {
|
||||||
throw new Error(`Feature: ${feature} is not a boolean`)
|
throw new Error(`Feature: ${feature} is not a boolean`)
|
||||||
}
|
}
|
||||||
|
|
||||||
// @ts-ignore
|
// @ts-expect-error - TS does not like you writing into a generic type,
|
||||||
flags[feature] = value
|
// but we know that it's okay in this case because it's just an object.
|
||||||
|
flagValues[feature] = value
|
||||||
tags[`flags.${feature}.source`] = "environment"
|
tags[`flags.${feature}.source`] = "environment"
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -159,19 +154,22 @@ export class FlagSet<V extends Flag<any>, T extends { [key: string]: V }> {
|
||||||
const license = ctx?.user?.license
|
const license = ctx?.user?.license
|
||||||
if (license) {
|
if (license) {
|
||||||
for (const feature of license.features) {
|
for (const feature of license.features) {
|
||||||
const flag = this.flags[feature]
|
if (!this.isFlagName(feature)) {
|
||||||
if (!flag) {
|
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
if (flags[feature] === true || specificallySetFalse.has(feature)) {
|
if (
|
||||||
|
flagValues[feature] === true ||
|
||||||
|
specificallySetFalse.has(feature)
|
||||||
|
) {
|
||||||
// If the flag is already set to through environment variables, we
|
// If the flag is already set to through environment variables, we
|
||||||
// don't want to override it back to false here.
|
// don't want to override it back to false here.
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
// @ts-ignore
|
// @ts-expect-error - TS does not like you writing into a generic type,
|
||||||
flags[feature] = true
|
// but we know that it's okay in this case because it's just an object.
|
||||||
|
flagValues[feature] = true
|
||||||
tags[`flags.${feature}.source`] = "license"
|
tags[`flags.${feature}.source`] = "license"
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -180,25 +178,26 @@ export class FlagSet<V extends Flag<any>, T extends { [key: string]: V }> {
|
||||||
if (posthog && identity?.type === IdentityType.USER) {
|
if (posthog && identity?.type === IdentityType.USER) {
|
||||||
const posthogFlags = await posthog.getAllFlagsAndPayloads(identity._id)
|
const posthogFlags = await posthog.getAllFlagsAndPayloads(identity._id)
|
||||||
for (const [name, value] of Object.entries(posthogFlags.featureFlags)) {
|
for (const [name, value] of Object.entries(posthogFlags.featureFlags)) {
|
||||||
const flag = this.flags[name]
|
if (!this.isFlagName(name)) {
|
||||||
if (!flag) {
|
|
||||||
// We don't want an unexpected PostHog flag to break the app, so we
|
// We don't want an unexpected PostHog flag to break the app, so we
|
||||||
// just log it and continue.
|
// just log it and continue.
|
||||||
console.warn(`Unexpected posthog flag "${name}": ${value}`)
|
console.warn(`Unexpected posthog flag "${name}": ${value}`)
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
if (flags[name] === true || specificallySetFalse.has(name)) {
|
if (flagValues[name] === true || specificallySetFalse.has(name)) {
|
||||||
// If the flag is already set to through environment variables, we
|
// If the flag is already set to through environment variables, we
|
||||||
// don't want to override it back to false here.
|
// don't want to override it back to false here.
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
const payload = posthogFlags.featureFlagPayloads?.[name]
|
const payload = posthogFlags.featureFlagPayloads?.[name]
|
||||||
|
const flag = this.flagSchema[name]
|
||||||
try {
|
try {
|
||||||
// @ts-ignore
|
// @ts-expect-error - TS does not like you writing into a generic
|
||||||
flags[name] = flag.parse(payload || value)
|
// type, but we know that it's okay in this case because it's just
|
||||||
|
// an object.
|
||||||
|
flagValues[name] = flag.parse(payload || value)
|
||||||
tags[`flags.${name}.source`] = "posthog"
|
tags[`flags.${name}.source`] = "posthog"
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
// We don't want an invalid PostHog flag to break the app, so we just
|
// We don't want an invalid PostHog flag to break the app, so we just
|
||||||
|
@ -208,12 +207,12 @@ export class FlagSet<V extends Flag<any>, T extends { [key: string]: V }> {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
for (const [key, value] of Object.entries(flags)) {
|
for (const [key, value] of Object.entries(flagValues)) {
|
||||||
tags[`flags.${key}.value`] = value
|
tags[`flags.${key}.value`] = value
|
||||||
}
|
}
|
||||||
span?.addTags(tags)
|
span?.addTags(tags)
|
||||||
|
|
||||||
return flags
|
return flagValues
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue