Skip to content

Conversation

@duyhungtnn
Copy link
Collaborator

@duyhungtnn duyhungtnn commented Sep 10, 2025

This PR initializes a new OpenFeature provider for Bucketeer, creating a Node.js server SDK that bridges the Bucketeer feature flag service with OpenFeature's standardized API.

@duyhungtnn duyhungtnn self-assigned this Sep 10, 2025
@duyhungtnn duyhungtnn requested a review from Copilot September 10, 2025 12:55
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR initializes a new OpenFeature provider for Bucketeer, creating a Node.js server SDK that bridges the Bucketeer feature flag service with OpenFeature's standardized API.

  • Sets up TypeScript project configuration with linting, formatting, and build tools
  • Implements BuckeeterProvider class that wraps the Bucketeer SDK to provide OpenFeature-compatible flag evaluation
  • Creates utility functions for converting between OpenFeature evaluation contexts and Bucketeer user objects

Reviewed Changes

Copilot reviewed 14 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
package.json Defines project metadata, dependencies, and peer dependencies for the OpenFeature Bucketeer provider
src/internal/BucketeerProvider.ts Main provider implementation with flag evaluation methods and client initialization
src/internal/EvaluationContext.ts Utility functions for converting OpenFeature contexts to Bucketeer user objects
src/internal/BKTEvaluationDetailExt.ts Helper functions to convert Bucketeer evaluation details to OpenFeature resolution details
src/internal/version.ts Exports SDK version from package.json
src/index.ts Main entry point exporting the provider and re-exporting Bucketeer SDK types
tsconfig.test.json TypeScript configuration for test files
tslint.json TSLint configuration extending basic rules
eslint.config.mjs ESLint configuration for TypeScript files
.prettierrc.js Code formatting configuration
.prettierignore Files to exclude from Prettier formatting
.nvmrc Node.js version specification
.npmrc NPM registry configuration
.npmignore Files to exclude from NPM package

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@duyhungtnn duyhungtnn requested a review from Copilot September 11, 2025 02:21
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 14 out of 16 changed files in this pull request and generated 3 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@duyhungtnn duyhungtnn requested a review from Copilot September 12, 2025 08:20
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 16 out of 19 changed files in this pull request and generated 3 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@duyhungtnn duyhungtnn requested a review from Copilot September 12, 2025 09:01
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 15 out of 18 changed files in this pull request and generated 2 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@duyhungtnn duyhungtnn marked this pull request as ready for review September 12, 2025 09:12
@duyhungtnn duyhungtnn requested a review from Copilot September 12, 2025 10:43
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 18 out of 22 changed files in this pull request and generated 5 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@duyhungtnn duyhungtnn requested a review from Copilot September 12, 2025 10:48
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 18 out of 22 changed files in this pull request and generated 3 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@duyhungtnn
Copy link
Collaborator Author

duyhungtnn commented Oct 2, 2025

Updated

  • Document in README
  • E2e tests
  • Example
  • Github workflows
    • e2e
    • pull_request

@duyhungtnn duyhungtnn requested a review from Copilot October 2, 2025 09:14
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 35 out of 40 changed files in this pull request and generated no new comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Replaces default import of dotenv with named import and updates usage in bucketeer-config.ts. Adds fallback values for userAgent and ipAddress in flags route. Fixes tsconfig.json structure and adds allowSyntheticDefaultImports option.

feat: init SDK

feat: added detail implementation

chore: edit parameter names in BucketeerProvider methods

Replaces underscored parameter names with direct names in evaluation methods for clarity and consistency. Updates internal references to use the new parameter names.

fix: assign client instance in BucketeerProvider

Sets the initialized client to `this.client` in BucketeerProvider to ensure the instance is available after initialization. Also removes the empty 'exclude' array from tsconfig.test.json.

chore: Reset client to undefined on close

After destroying the client in onClose, set the client property to undefined to ensure proper cleanup and prevent potential reuse of a destroyed client instance.

chore: switch test runner from Ava to Jest

Replaced Ava and related packages with Jest, ts-jest, and @types/jest in package.json. Updated tsconfig.test.json to remove unused types. This change standardizes testing on Jest for better TypeScript integration and ecosystem support.

test: add Jest test setup and initial unit tests

Introduces Jest configuration and scripts for testing. Adds unit tests for BKTEvaluationDetailExt and EvaluationContext utilities. Updates TypeScript config to include test files and removes tsconfig.test.json. Cleans up package.json scripts and dependencies.

test: init provider test

test: evaluation logic

chore: Use constant for initialization timeout

Replaces hardcoded timeout value with DEFAULT_WAIT_FOR_INITIALIZATION_TIMEOUT_MS in BuckeeterProvider and updates related test to use the constant. This improves maintainability and consistency.

fix: use SDK version import in version.ts

Replaces require with ES module import for package.json to improve consistency and compatibility with modern TypeScript/ES module syntax.

chore: add initialization timeout option to BuckeeterProvider

BuckeeterProvider now accepts an optional initializationTimeoutMs parameter, allowing customization of the client initialization timeout. Updated related tests to cover the new option and default behavior. Also removed test files from tsconfig.json 'include' and deleted tslint.json.

fix: ESLint config: use 'extends' instead of 'plugins'

Replaces the incorrect 'plugins' key with 'extends' in the ESLint configuration to properly apply recommended TypeScript and Prettier rules.

chore: update import syntax for package.json

Changed the import statement for package.json to use default import syntax, improving compatibility with module systems.

fix: lint fail in tests by using explicit undefined variable

Replaces direct usage of 'undefined' with an explicit variable cast to 'any' in test cases for convertContextValueToString and evaluationContextToBKTUser. This improves clarity and type safety in the test suite.

chore: improve config validation in BucketeerProvider

Constructor now uses defineBKTConfig to validate and fill defaults for user-provided config. This ensures missing required fields are caught early, providing faster feedback to developers. Also, wrapper SDK version and source ID are always set, overriding user values.

test: update BucketeerProvider test config initialization

Refactored test configuration to use defineBKTConfig for both mockConfig and expectedConfig. Updated interval values to milliseconds and incremented appVersion to 1.0.1 for consistency with the new config structure.

chore: enable lint check

chore: add PR title validation and CI workflows

Introduces GitHub Actions workflows for validating pull request titles against semantic conventions and for running CI tasks including dependency installation, linting, unit tests, and build on PRs targeting the main branch.

Update .prettierignore to exclude more directories

Expanded the .prettierignore file to ignore common build, coverage, and dependency directories, including .github/, node_modules/, dist/, build/, coverage/, and their equivalents in example subdirectories.

fix: build command

fix(typo): Rename BuckeeterProvider to BucketeerProvider

Corrects the class name, import/export statements, and test references from 'BuckeeterProvider' to 'BucketeerProvider' for consistency and accuracy.

chore: init example

Update .gitignore

feat: init example

chore: update make_file

test: init e2e test

test: add dotenv support and update SDK dependencies

Introduced .env.example and dotenv for environment variable management. Updated constants to use new variable names, added jest.setup.js for dotenv initialization, and configured Jest to use the setup file. Upgraded bkt-node-server-sdk to v0.4.2 and added dotenv to devDependencies.

fix: BucketeerProvider tests for config handling

Simplifies test variable naming and improves assertions to verify that SDK version and source ID are set internally, regardless of user config input. Removes redundant expectedConfig and clarifies the initialization logic in BucketeerProvider tests.

test(e2e): evaluation test cases

The BucketeerProvider now includes the SDK version in its metadata. Also, SDK_VERSION is exported from the main entry point. Added comprehensive e2e tests for feature flag evaluation covering boolean, string, number, and JSON types.

fix: update e2e test case

chore: add local evaluation tests and rename remote tests

Introduced localEvaluate.test.ts to test BucketeerProvider with local evaluation enabled. Renamed evaluate.test.ts to remoteEvaluate.test.ts and added logger to the BKTConfig in the remote evaluation tests for consistency.

Create e2e.yml

fix: lint fail because unused file

Update pull-request.yml

fixf: formatting in example project files

Added a missing newline in tsconfig.json and package.json, and improved spacing in README.md for better readability and consistency.

Update README and flags route with improved examples

Expanded the README with comprehensive usage instructions, examples, and documentation for integrating Bucketeer with OpenFeature in Node.js. Refactored the example flags route to use request headers for evaluation context and return multiple feature flag values, aligning with the new documentation.

Update README.md

chore: remove push trigger for feat/example branch in workflows

The push event for the 'feat/example' branch has been removed from both e2e.yml and pull-request.yml workflows, leaving only workflow_dispatch and workflow_call triggers. This streamlines workflow execution and prevents unnecessary runs on pushes to the example feature branch.

chore: move and update imports in e2e test

Renamed localEvaluate.test.ts and remoteEvaluate.test.ts from the constants directory to the e2e root. Updated import paths to reference './constants/constants' instead of './constants'.

fix: correct context

Updated README and e2e tests to import BucketeerProvider and defineBKTConfig from '@bucketeer/openfeature-node-server-sdk' instead of 'bkt-node-server-sdk'. Refactored tests to explicitly pass evaluation context to feature flag evaluation methods, improving clarity and aligning with OpenFeature best practices.

fix: handle missing case when resolving object with null

Refactor type mismatch object evaluation tests

Consolidated multiple individual tests for type mismatches in object evaluation into a single parameterized test suite. This improves maintainability and reduces code duplication in BucketeerProvider tests.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 35 out of 42 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@nnnkkk7 nnnkkk7 left a comment

Choose a reason for hiding this comment

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

Thanks!
LGTM!!

@duyhungtnn
Copy link
Collaborator Author

Hi @cre8ivejp, @nnnkkk7,
I think there’s an issue in the objectVariationDetails method implementation.
The provider throws an error when returning a default evaluation where evaluation_value is not an object.

For example:

  • If the user sets the default value to null and there’s no evaluation for the flag key,
  • our SDK returns the default evaluation using the user’s default value — in this case, variationValue will be null.
  • null is a valid type for both OpenFeature JsonValue and our SDK’s BKTValue.
  • So Our Provider throw an error, but it should not.

Please do not merge this PR, I will make a fix.

@duyhungtnn duyhungtnn marked this pull request as draft October 30, 2025 14:21
Updated BucketeerProvider to accept all valid JsonValue types, including null and primitives, in objectVariationDetails. Adjusted related tests to reflect the new behavior and removed type mismatch error handling for non-object values.
@duyhungtnn duyhungtnn requested a review from Copilot October 31, 2025 03:17
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 35 out of 42 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@duyhungtnn duyhungtnn requested a review from nnnkkk7 October 31, 2025 03:18
@duyhungtnn
Copy link
Collaborator Author

I updated the PR 2cc7966
Please help me to take a look @nnnkkk7

@duyhungtnn duyhungtnn marked this pull request as ready for review October 31, 2025 03:24
Copy link
Contributor

@nnnkkk7 nnnkkk7 left a comment

Choose a reason for hiding this comment

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

LGTM!
Thank you!

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.

Amazing work!
Thank you 🎉

@cre8ivejp cre8ivejp changed the title feat: init provider feat: add provider implementation Nov 17, 2025
@cre8ivejp cre8ivejp merged commit 18e1dea into main Nov 17, 2025
4 checks passed
@cre8ivejp cre8ivejp deleted the feat/init-sdk branch November 17, 2025 01:58
@cre8ivejp cre8ivejp mentioned this pull request Nov 19, 2025
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.

4 participants