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

feat: get variation details by variation type #155

Merged
merged 89 commits into from
Sep 9, 2024

Conversation

duyhungtnn
Copy link
Collaborator

@duyhungtnn duyhungtnn commented Jun 24, 2024

Related bucketeer-io/bucketeer#886

Changes

  • Added new type BKTValue to hold all valid types that could used on the interface. (refs from OpenFeature SDK interface)
export type BKTPrimitiveValue = null | boolean | string | number
export type BKTJsonObject = {
  [key: string]: BKTValue
}
export type BKTJsonArray = BKTValue[]
/**
 * Represents a JSON node value.
 */
export type BKTValue = BKTPrimitiveValue | BKTJsonObject | BKTJsonArray
  • Create the new data class BKTEvaluationDetail<BKTValue> to support getting variation details by type.
  • Introduce a new interface to get variation details by variation type
  jsonVariation: (
    featureId: string,
    defaultValue: BKTJsonValue,
  ) => BKTJsonValue
   objectVariation: (
    featureId: string,
    defaultValue: BKTJsonValue,
  ) => BKTJsonValue
   stringVariationDetails: (
    featureId: string,
    defaultValue: string,
  ) => BKTEvaluationDetail<string>

  numberVariationDetails: (
    featureId: string,
    defaultValue: number,
  ) => BKTEvaluationDetail<number>

  booleanVariationDetails: (
    featureId: string,
    defaultValue: boolean,
  ) => BKTEvaluationDetail<boolean>
  
   objectVariationDetails: (
    featureId: string,
    defaultValue: BKTJsonValue,
  ) => BKTEvaluationDetail<BKTJsonValue>
  • The method evaluationDetails will be deprecated after this PR.

Breaking changes

  • This PR introduces breaking changes on the method jsonVariation. The previous logic could return any type but we don't allow it anymore.
    The returned value will be either a BKTJsonObject or a BKTJsonArray. If no result is found, it will return the provided defaultValue, which can be of any type within BKTJsonValue.

  • Also this method jsonVariation is marked as deprecated and the user should use objectVariation instead.
    It is not removed to prevent build failures in the current project that uses this method.

@cre8ivejp cre8ivejp changed the title feat: init new interface feat: get variation details by variation type Jun 26, 2024
test/BKTClient.spec.ts Show resolved Hide resolved
test/BKTClient.spec.ts Outdated Show resolved Hide resolved
src/BKTClient.ts Outdated Show resolved Hide resolved
@duyhungtnn duyhungtnn marked this pull request as ready for review July 3, 2024 06:28
@cre8ivejp
Copy link
Member

@duyhungtnn, please fix the conflicts.

@duyhungtnn duyhungtnn force-pushed the feat/new-detailed-evaluation-interface branch from 984576f to 31982a5 Compare July 19, 2024 04:00
@duyhungtnn
Copy link
Collaborator Author

@cre8ivejp I fixed the conflict and updated this PR with new changes from the main branches.

@duyhungtnn
Copy link
Collaborator Author

duyhungtnn commented Aug 2, 2024

  • The jsonVariation method has been deprecated. It is not removed to prevent build failures in the current project that uses this method.

src/BKTClient.ts Outdated Show resolved Hide resolved
src/BKTClient.ts Outdated Show resolved Hide resolved
src/BKTClient.ts Outdated Show resolved Hide resolved
src/JsonTypes.ts Outdated Show resolved Hide resolved
src/JsonTypes.ts Outdated
Copy link
Member

Choose a reason for hiding this comment

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

WDYT of renaming the file name to BKTJsonTypes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it could better if named as BKTValue like Android & iOS

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I updated it 14c24ac

@duyhungtnn
Copy link
Collaborator Author

@duyhungtnn duyhungtnn requested a review from cre8ivejp September 6, 2024 03:45
Copy link
Member

@cre8ivejp cre8ivejp left a comment

Choose a reason for hiding this comment

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

Thank you!

@cre8ivejp cre8ivejp merged commit d3f76e2 into main Sep 9, 2024
3 checks passed
@cre8ivejp cre8ivejp deleted the feat/new-detailed-evaluation-interface branch September 9, 2024 01:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants