-
Notifications
You must be signed in to change notification settings - Fork 2
feat: support for the React Native environment #223
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
Conversation
There was a problem hiding this 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 adds support for the React Native environment by updating the SDK’s initialization flow and configuration along with new and updated tests for native and node contexts.
- Adds a native entry point (src/main.native.ts) that enforces an idGenerator via assertIdGenerator.
- Introduces a DummyPlatformModule and updates platform module selection in both node and React Native environments.
- Updates configuration (BKTConfig) to optionally accept an idGenerator and adjusts various test and e2e files accordingly.
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
vitest-e2e-node.config.ts | New node configuration with embedded SDK version. |
test/main.native.spec.ts | Tests for React Native environment ensuring proper idGenerator handling. |
src/main.ts | Updated platform module selection based on idGenerator presence for Node. |
src/main.native.ts | Added React Native entry point with mandatory idGenerator enforcement. |
src/internal/di/PlatformModule.ts | New BasePlatformModule and DummyPlatformModule implementations added. |
src/BKTConfig.ts | Extended configuration to optionally support idGenerator. |
e2e/node/BKTClient.spec.ts | End-to-end tests for Node environment with updated logic. |
e2e/browser/events.spec.ts | Import path updates for browser events tests. |
e2e/browser/evaluations.spec.ts | Import path updates for browser evaluations tests. |
e2e/browser/BKTClient.spec.ts | Import path updates and revised test setup for browser client tests. |
Files not reviewed (1)
- package.json: Language not supported
c5751a0
to
316f045
Compare
There was a problem hiding this 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 adds support for the React Native environment by introducing a new native entry point while updating configurations and tests for multiple environments.
- Adds a new React Native initialization file (src/main.native.ts) that enforces the presence of an idGenerator.
- Updates configuration and DI logic to conditionally use BasePlatformModule when an idGenerator is provided.
- Adjusts e2e tests and environment setups (both browser and node) to use environment-appropriate fetch providers and storage handling.
Reviewed Changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
vitest-e2e.config.ts | Adds browser-specific setup file to support e2e tests in happy-dom environment. |
vitest-e2e-node.config.ts | Introduces a new e2e config for node with proper setup files. |
test/main.native.spec.ts | Adds tests ensuring requiredIdGenerator behavior in the native environment. |
src/main.ts | Modifies component creation to conditionally use BasePlatformModule when idGenerator is set. |
src/main.native.ts | New file providing React Native initialization using requiredIdGenerator. |
src/internal/di/PlatformModule.ts | Adds BasePlatformModule implementation for environments where an idGenerator is provided. |
src/BKTConfig.ts | Updates configuration to optionally include an idGenerator. |
e2e/**/*.ts | Adjusts various tests to rely on fetchLike and conditionally clear localStorage. |
e2e/environment.ts | Provides functions to set fetch provider and environment flags. |
e2e/BKTClient.spec.ts | Updates tests to use fetchLike and remove redundant localStorage clear calls. |
Files not reviewed (1)
- package.json: Language not supported
Comments suppressed due to low confidence (2)
src/internal/di/PlatformModule.ts:10
- [nitpick] Consider renaming '_idGenerator' to 'idGenerator' to simplify the property name, unless the underscore prefix is a deliberate convention for indicating a protected member.
protected _idGenerator: IdGenerator
e2e/events.spec.ts:177
- [nitpick] Consider abstracting the localStorage clearing logic into a helper function to reduce repetition and improve maintainability in your test cleanup code.
if (typeof localStorage !== 'undefined') { localStorage.clear() }
e9284fd
to
116c420
Compare
116c420
to
f56b14b
Compare
f29d97d
to
b0dcaa8
Compare
@cre8ivejp please help me to take a look. |
fix: support react native SDK fix: test fail on node:e2e test: remove duplicate/ unnecessary localStorage.clear() Update events.spec.ts Update events.spec.ts test: inject correct entry point for node environment Update package.json Update package.json fix: lint fail chore: required id-generator for react native environment bump to 2.2.13 fix: missing default user agent in react native environment Update package.json
0259869
to
346bd91
Compare
if ( | ||
typeof window !== 'undefined' && | ||
window.navigator && | ||
typeof window.navigator.userAgent === 'string' | ||
) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on react-native
environment, we have window
!= undefined
but the navigator
is undefined
.
There was a problem hiding this 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 adds React Native support to the JavaScript client SDK by introducing environment-specific entry points and configuration validation. The implementation ensures React Native environments provide required dependencies while maintaining compatibility with existing browser and Node.js environments.
Key changes:
- Added React Native-specific module with mandatory
idGenerator
validation - Updated package.json exports to include React Native entry points
- Refactored e2e tests to support multiple environments with shared test logic
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
src/main.native.ts | New React Native entry point with idGenerator validation |
src/BKTConfig.ts | Added idGenerator to configuration interface and improved user agent detection |
src/internal/di/PlatformModule.ts | Added BasePlatformModule for dependency injection |
package.json | Added React Native exports and updated test scripts |
e2e/ files | Refactored tests for environment-agnostic execution |
Comments suppressed due to low confidence (1)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work 🚀
This PR adds support for the React Native environment by updating the SDK’s initialization flow and configuration along with new and updated tests for native and node contexts.
Please merge this PR #226 before merge this PR.