feat: unify initialization process with other sdks#106
Merged
Conversation
duyhungtnn
changed the title
Collaborator
Author
|
@cre8ivejp please help me to take a look |
Collaborator
Author
|
I will put the changes for refactoring the initialization process in this PR. SO I converted this PR back to a draft |
duyhungtnn
changed the title
duyhungtnn
force-pushed
the
fix/no-spead-after-defaults-lint
branch
2 times, most recently
from
8636734 to
a8342d6
Compare
Introduces a custom ESLint rule to disallow spreading objects after default properties in object literals, preventing accidental override of defaults with undefined values. Includes rule implementation, documentation, tests, and integration into the ESLint config.
Replaced object spread config merging with explicit property assignment in defineBKTConfig for improved clarity and type safety. Updated initialize to use defineBKTConfig. Added a Makefile target and workflow step for running a custom ESLint rule test.
Corrected 'reafactor' to 'refactor' in the comment about future deprecation and refactoring of the initialize function.
Introduces the sdkVersion parameter to feature flag and segment user cache processors, as well as to event creation and registration logic. Updates all relevant tests and internal calls to ensure sdkVersion is consistently passed and handled throughout the codebase. This change improves traceability and compatibility for SDK versioning in event and cache operations.
Updated event test files to use nodeSDKVersion instead of version when checking the sdkVersion property. This ensures consistency with the updated import from the version module.
Added the sdkVersion parameter to relevant test cases and updated interface property formatting in config and internalConfig for consistency. Also added TODO comments for missing tests in metricsEvent functions and removed an unnecessary blank line in sourceId.
Corrects the call to createInternalSdkErrorMetricsEvent in toErrorMetricsEvent to include sdkVersion as an argument. Also removes an unused convertMS function and a TODO comment.
Replaces the createBasicConfig helper with a baseConfig constant and updates test cases to use more explicit type casting and object spreading. This improves readability, reduces indirection, and ensures better type safety in test setup.
Added logger warnings when config values are invalid and default values are used in config.ts. Improved code formatting and consistency in both config.ts and internalConfig.ts, including semicolon usage and export statements. No functional changes to logic except for added logging.
Fixed minor formatting issues in sourceId type and function definitions, and corrected a comment typo in config.ts. No functional changes were made.
Introduces a type-check step to the pull request workflow using the Makefile. Updates ESLint config to include additional test directories and removes 'symbol-observable' from test TypeScript config types.
Refactored BKTConfig and related code to replace the 'pollingInterval' property with 'cachePollingInterval' for clarity. Updated all usages, validation logic, and tests to reflect the new property name. Also added 'skipLibCheck' to TypeScript configs for improved build performance.
Both feature flag and segment users cache processors now execute cache update immediately upon start, in addition to scheduled polling. Unit tests have been updated to reflect the new polling and event emission behavior.
Introduces a 'test-single' Makefile target to run individual test files. Updates README with instructions for running all tests and single tests using the new Makefile commands.
Updated cachePollingInterval from 1000/3000ms to 15000ms in all local evaluation test files to reduce polling frequency. This change may help decrease load on the API and improve test stability.
Refactored the pull request workflow to separate linting and custom ESLint rule tests. Updated various dependencies in package.json to newer versions for improved stability and compatibility.
Replaces version tags with commit SHAs for actions/checkout, actions/setup-node, and actions/cache to improve security and reliability. Also updates step names for clarity in the workflow.
duyhungtnn
force-pushed
the
fix/no-spead-after-defaults-lint
branch
from
05f5303 to
d268fbb
Compare
Updated the import path for FEATURE_FLAG_REQUESTED_AT in polling.ts and update.ts test files to reference the correct location. This resolves incorrect import errors due to a previous path typo.
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces a comprehensive refactoring of the SDK initialization process to improve modularity and support for wrapper SDKs like OpenFeature. It includes a new configuration approach, better source ID management, and enhanced type safety while maintaining backward compatibility.
Key changes:
- Introduces new
BKTConfiginterface anddefineBKTConfigfunction for improved configuration management - Adds support for wrapper SDK source ID and version tracking for better telemetry
- Refactors internal initialization to use
InternalConfigwith proper validation
Reviewed Changes
Copilot reviewed 52 out of 53 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/config.ts | Introduces new BKTConfig interface and defineBKTConfig function |
| src/internalConfig.ts | New internal configuration management with validation |
| src/index.ts | Adds new initializeBKTClient function and maintains backward compatibility |
| src/objects/sourceId.ts | Converts enum to const object with helper functions |
| src/objects/version.ts | Renames version export to nodeSDKVersion |
| Multiple event files | Updates event creation to accept sourceId and sdkVersion parameters |
Comments suppressed due to low confidence (1)
src/objects/status.ts:1
- [nitpick] Property shorthand can be used here. These can be simplified to
sourceId,andsdkVersion,since the property names match the variable names.
import { ApiId, NodeApiIds } from './apiId';
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Deleted 'skipLibCheck' and 'pretty' options from tsconfig.json and tsconfig.test.json to simplify configuration and remove unnecessary settings.
Corrected the comment from 'cachPollingInterval' to 'cachePollingInterval' for clarity in the config validation logic.
cre8ivejp
changed the title
cre8ivejp
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces
wrapperSDKVersionandwrapperSourceIdfor the Node OpenFeature Providerrefs: bucketeer-io/javascript-client-sdk#236