Update Browser SDK #713
Update Browser SDK #713
Conversation
🦋 Changeset detectedLatest commit: f3734c8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThe changes in this pull request primarily involve modifications to environment configuration management, client creation, and type definitions across several files. The Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Deploying xmtp-js-docs with
|
| Latest commit: |
f3734c8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a9167fb4.xmtp-js.pages.dev |
| Branch Preview URL: | https://rygine-updates.xmtp-js.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (12)
examples/react-vite-browser-sdk/src/main.tsx (1)
12-15: Enhance environment variable validationWhile the validation is good, consider these improvements for better maintainability and type safety:
+// Create a type-safe config object +interface EnvConfig { + projectId: string; +} + +// Centralize environment validation +const getEnvConfig = (): EnvConfig => { + const projectId = import.meta.env.VITE_PROJECT_ID; + if (!projectId) { + throw new Error( + "VITE_PROJECT_ID environment variable is required for RainbowKit configuration" + ); + } + return { projectId }; +}; + -const projectId = import.meta.env.VITE_PROJECT_ID; -if (!projectId) { - throw new Error("VITE_PROJECT_ID must be set in the environment"); -} +const { projectId } = getEnvConfig();examples/react-vite-browser-sdk/src/createClient.ts (2)
Line range hint
25-35: Add error handling for client operations.The client creation and registration process lacks proper error handling.
Consider wrapping operations in try-catch:
+ try { const client = await Client.create(wallet.account.address, encryptionBytes, { env: import.meta.env.VITE_XMTP_ENVIRONMENT || "production", }); const isRegistered = await client.isRegistered(); if (!isRegistered) { const signature = await getSignature(client, wallet); if (signature) { await client.addSignature(SignatureRequestType.CreateInbox, signature); } await client.registerIdentity(); } return client; + } catch (error) { + throw new Error( + `Failed to create or register XMTP client: ${(error as Error).message}` + ); + }
16-16: Update function signature to match implementation.The function signature doesn't reflect that
encryptionKeyis now handled internally.The current implementation handles the encryption key via environment variables, so the signature should remain unchanged. However, consider adding JSDoc to clarify the requirements:
+ /** + * Creates and registers an XMTP client + * @param walletKey - The wallet key for authentication + * @throws {Error} If VITE_ENCRYPTION_KEY environment variable is not set or invalid + * @returns {Promise<Client>} The created and registered XMTP client + */ export const createClient = async (walletKey: string) => {sdks/browser-sdk/src/utils/createClient.ts (2)
30-34: Consider simplifying the logging checkThe logging check could be more concise using object destructuring and nullish coalescing.
- const isLogging = - options && - (options.loggingLevel !== undefined || - options.structuredLogging || - options.performanceLogging); + const { loggingLevel, structuredLogging, performanceLogging } = options ?? {}; + const isLogging = loggingLevel !== undefined || structuredLogging || performanceLogging;
Line range hint
20-24: Address the TODO comment for database path validationThe TODO comment about database path validation should be addressed to prevent potential security issues.
Would you like me to help implement the database path validation or create a GitHub issue to track this task?
packages/frames-validator/package.json (1)
41-41: LGTM! Consider documenting the dependency.The addition of
@types/bltypes package is appropriate. However, it would be helpful to document why these buffer list types are needed, either in the PR description or in the relevant code that uses them.sdks/browser-sdk/package.json (1)
Line range hint
1-99: Consider adding lockfile for dependency managementThe package.json doesn't seem to be accompanied by a package-lock.json or yarn.lock file in the changes. Given the early versions of dependencies like wasm-bindings (0.0.x), it's crucial to lock dependency versions to ensure consistent builds.
sdks/browser-sdk/test/helpers.ts (1)
14-14: Consider making the encryption key deterministic for testsWhile using
crypto.getRandomValuesis secure, having random values in tests can make them non-deterministic and harder to debug. Consider using a fixed test key or making it configurable through environment variables.-const testEncryptionKey = window.crypto.getRandomValues(new Uint8Array(32)); +// Use a fixed key for predictable test behavior +const testEncryptionKey = new Uint8Array(32).fill(1); +// Or make it configurable +const testEncryptionKey = process.env.TEST_ENCRYPTION_KEY + ? hexToBytes(process.env.TEST_ENCRYPTION_KEY) + : new Uint8Array(32).fill(1);sdks/browser-sdk/src/WorkerClient.ts (1)
26-30: LGTM! Consider adding JSDoc documentation.The addition of the required
encryptionKeyparameter aligns with the PR objectives and is correctly implemented. However, adding JSDoc documentation would help developers understand the encryption key requirements and format.Add documentation like:
+/** + * Creates a new WorkerClient instance + * @param accountAddress - The account address + * @param encryptionKey - The encryption key as Uint8Array + * @param options - Optional client configuration + * @returns A new WorkerClient instance + */ static async create( accountAddress: string, encryptionKey: Uint8Array, options?: Omit<ClientOptions, "codecs">, )sdks/browser-sdk/src/types/clientEvents.ts (1)
82-92: Consider adding JSDoc comments for the new SCW signature action.The new
addScwSignatureaction type is well-structured with appropriate blockchain-specific fields. Consider adding JSDoc comments to document the purpose and usage of this action type, similar to other action types in the file.+ /** + * Adds a smart contract wallet signature with blockchain-specific context + * @property {SignatureRequestType} type - Type of signature request + * @property {Uint8Array} bytes - Signature data + * @property {bigint} chainId - Target blockchain network identifier + * @property {bigint} [blockNumber] - Optional block number for transaction context + */ | { action: "addScwSignature"; id: string; result: undefined; data: { type: SignatureRequestType; bytes: Uint8Array; chainId: bigint; blockNumber?: bigint; }; }sdks/browser-sdk/src/workers/client.ts (1)
124-136: LGTM! Consider adding validation for chainId and blockNumber.The implementation properly handles smart contract wallet signatures with all necessary parameters.
Consider adding these improvements:
- Validate chainId against a list of supported chains
- Add bounds checking for blockNumber
- Consider adding a timeout mechanism for very old block numbers
Example validation:
if (!SUPPORTED_CHAIN_IDS.includes(data.chainId)) { throw new Error(`Unsupported chain ID: ${data.chainId}`); } if (data.blockNumber <= 0) { throw new Error("Block number must be positive"); }sdks/browser-sdk/src/Client.ts (1)
51-53: Simplify the logging level condition for readabilityThe condition used to determine the logging level in the
supercall can be simplified to improve readability:options?.loggingLevel !== undefined && options.loggingLevel !== "off"Consider simplifying the condition as follows:
super( worker, - options?.loggingLevel !== undefined && options.loggingLevel !== "off", + options?.loggingLevel && options.loggingLevel !== "off", );This modification maintains the same logic while making the condition more concise.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (19)
.gitignore(1 hunks)examples/react-vite-browser-sdk/.env.example(1 hunks)examples/react-vite-browser-sdk/src/createClient.ts(1 hunks)examples/react-vite-browser-sdk/src/globals.d.ts(1 hunks)examples/react-vite-browser-sdk/src/main.tsx(1 hunks)packages/frames-validator/package.json(1 hunks)packages/frames-validator/tsconfig.json(1 hunks)sdks/browser-sdk/package.json(1 hunks)sdks/browser-sdk/rollup.config.js(1 hunks)sdks/browser-sdk/src/Client.ts(3 hunks)sdks/browser-sdk/src/WorkerClient.ts(2 hunks)sdks/browser-sdk/src/WorkerConversation.ts(2 hunks)sdks/browser-sdk/src/index.ts(1 hunks)sdks/browser-sdk/src/types/clientEvents.ts(2 hunks)sdks/browser-sdk/src/types/options.ts(1 hunks)sdks/browser-sdk/src/utils/conversions.ts(1 hunks)sdks/browser-sdk/src/utils/createClient.ts(2 hunks)sdks/browser-sdk/src/workers/client.ts(2 hunks)sdks/browser-sdk/test/helpers.ts(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- examples/react-vite-browser-sdk/.env.example
🔇 Additional comments (24)
packages/frames-validator/tsconfig.json (1)
3-4: LGTM! TypeScript configuration changes follow best practices.
The changes improve the TypeScript configuration by:
- Using the correct property name
"include"(instead of"includes") - Properly scoping included files to source and configuration files
- Explicitly excluding build artifacts and dependencies with
"exclude"
sdks/browser-sdk/src/index.ts (2)
4-5: LGTM! Improved export specificity for DecodedMessage
The change from wildcard exports to explicit type exports provides better clarity and control over the public API surface.
10-36: Verify the completeness of WASM bindings exports
The new exports from @xmtp/wasm-bindings align with the PR objective of upgrading WASM bindings and adding support for smart contract wallet signatures. However, let's verify that all necessary types are exported for the new functionality.
sdks/browser-sdk/src/types/options.ts (3)
40-46: LGTM: Enhanced logging capabilities
The addition of structured and performance logging options provides better debugging and monitoring capabilities.
47-50: LGTM: Granular logging levels
The replacement of enableLogging with specific logging levels provides better control over log verbosity.
Line range hint 1-58: Verify the impact of removing EncryptionOptions
The removal of EncryptionOptions type and moving encryptionKey to be a required parameter in client creation is a breaking change. Let's verify the migration path for existing consumers.
examples/react-vite-browser-sdk/src/main.tsx (2)
19-19: LGTM: Proper configuration usage
The projectId is correctly passed to RainbowKit's configuration.
Line range hint 1-40: Verify implementation of new SDK features
The PR objectives mention new features (encryption key requirement, WASM bindings, smart contract wallet signatures) that aren't demonstrated in this example file.
Let's check if these features are implemented elsewhere in the example:
examples/react-vite-browser-sdk/src/createClient.ts (1)
19-23: Verify encryption key security measures.
Let's ensure the encryption key is not being logged or exposed elsewhere in the codebase.
sdks/browser-sdk/rollup.config.js (2)
24-24: LGTM! External dependency correctly configured.
The addition of "@xmtp/content-type-group-updated" to the external dependencies is properly placed and aligns with the module bundling strategy.
24-24: Verify package dependency declaration.
Let's ensure this new external dependency is properly declared in package.json.
✅ Verification successful
Dependency Declaration Verified
The dependency "@xmtp/content-type-group-updated" is properly declared in package.json and exists on npm.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the new dependency is declared in package.json
# Check if the dependency exists in package.json
jq -r '.dependencies["@xmtp/content-type-group-updated"] // empty' sdks/browser-sdk/package.json
# Check if the package exists on npm
curl -s https://registry.npmjs.org/@xmtp/content-type-group-updated | jq -r '.["dist-tags"].latest // empty'
Length of output: 222
sdks/browser-sdk/src/utils/createClient.ts (3)
5-5: LGTM: LogOptions import is correctly placed
The addition of LogOptions import aligns with the new logging functionality implementation.
37-49: Clarify the purpose of the undefined parameter
The undefined parameter at line 42 needs documentation to explain its purpose. This will help maintainers understand why it's needed and what it represents.
#!/bin/bash
# Search for createWasmClient definition to understand parameter purpose
ast-grep --pattern 'createClient = function($PARAMS) { $BODY }' --lang ts11-13:
Breaking change: Add encryption key validation
The addition of a required encryptionKey parameter is a breaking change that will require updates to existing implementations. Consider adding validation to ensure the encryption key meets security requirements.
✅ Verification successful
Verified: All createClient usages include the required encryptionKey parameter.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for existing createClient usage to assess impact
rg -l "createClient\(" --type ts
# Search for any encryption key validation patterns in the codebase
ast-grep --pattern 'if ($CONDITION) { throw new Error($MESSAGE) }' --lang ts
Length of output: 48411
Script:
#!/bin/bash
# Correctly search for existing createClient usage to assess impact
rg -l "createClient\(" --type ts
Length of output: 277
Script:
#!/bin/bash
# Search for createClient calls without the encryptionKey parameter in the listed files
rg -L "createClient\([^)]*\bencryptionKey\b" \
sdks/browser-sdk/src/WorkerClient.ts \
sdks/browser-sdk/test/helpers.ts \
sdks/browser-sdk/test/Client.test.ts \
sdks/node-sdk/test/Client.test.ts \
sdks/node-sdk/src/Client.ts \
sdks/node-sdk/test/helpers.ts \
examples/react-vite-browser-sdk/src/App.tsx
Length of output: 405
packages/frames-validator/package.json (1)
41-41: Verify dependency version compatibility
Let's ensure the types version aligns with the actual bl package version and check for any security advisories.
✅ Verification successful
Dependency versions are compatible and secure
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify bl package version compatibility and security
# Check if bl is a direct or transitive dependency
rg '"bl":\s*"([^"]+)"' -g 'package.json'
# Check npm for latest versions and compatibility
curl -s "https://registry.npmjs.org/@types/bl" | jq '.["dist-tags"]'
curl -s "https://registry.npmjs.org/bl" | jq '.["dist-tags"]'
# Check for security advisories
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "bl") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
Length of output: 2212
sdks/browser-sdk/package.json (1)
68-68: Verify WASM bindings upgrade impact
The upgrade from ^0.0.3 to ^0.0.4 of @xmtp/wasm-bindings aligns with the PR objectives to support new features like encryption key requirements and smart contract wallet signatures. However, since this is an early version (0.0.x), we should carefully verify compatibility.
sdks/browser-sdk/src/WorkerClient.ts (1)
88-95: Implementation looks good! Verify SignatureRequestType usage.
The new addScwSignature method is well-implemented with proper parameter types and optional handling. It correctly supports the PR objective of adding smart contract wallet signatures.
Let's verify the SignatureRequestType usage:
sdks/browser-sdk/src/WorkerConversation.ts (3)
5-5: LGTM! Type import aligns with WASM bindings upgrade.
The addition of GroupMember type import is consistent with the PR's objective to upgrade WebAssembly bindings.
76-77: LGTM! Method naming convention update.
The change from snake_case to camelCase method names (creator_inbox_id() → creatorInboxId() and conversation_type() → conversationType()) improves API consistency.
82-82: Verify type compatibility with toSafeGroupMember utility.
The type cast to GroupMember[] aligns with the new type import, but let's verify the compatibility with the toSafeGroupMember utility function.
✅ Verification successful
Type compatibility confirmed with toSafeGroupMember utility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if toSafeGroupMember utility is updated to handle GroupMember type
# Search for toSafeGroupMember implementation
rg -A 10 "toSafeGroupMember.*=.*"
# Search for GroupMember type usage
ast-grep --pattern 'function toSafeGroupMember($_: GroupMember) { $$$ }'
Length of output: 966
Script:
#!/bin/bash
# Description: Modify ast-grep pattern to match arrow function definitions of toSafeGroupMember
# Updated ast-grep pattern for arrow function
ast-grep --pattern 'const toSafeGroupMember = ($_: GroupMember): SafeGroupMember => { $$$ }'
# Search for all usages of toSafeGroupMember to verify type handling
rg 'toSafeGroupMember\('
# Verify the definition of SafeGroupMember
ast-grep --pattern 'interface SafeGroupMember { $$$ }'
Length of output: 284
Script:
#!/bin/bash
# Description: Search for the definition of SafeGroupMember interface
# Use ripgrep to find the definition of SafeGroupMember
rg 'interface SafeGroupMember'
# If not found, search for type aliases
rg 'type SafeGroupMember'
# Additionally, search for export statements related to SafeGroupMember
rg 'export .*SafeGroupMember'
Length of output: 479
sdks/browser-sdk/src/utils/conversions.ts (1)
344-349: LGTM! Verify usage across the codebase.
The update to use GroupMember instead of WasmGroupMember aligns with the PR objectives of upgrading WASM bindings. The conversion maintains type safety through explicit property mapping.
Let's verify the usage of this conversion function across the codebase:
✅ Verification successful
Verified! No remaining references to WasmGroupMember and all usages of toSafeGroupMember are correctly updated.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all usages of toSafeGroupMember have been updated to use GroupMember type
# Search for any remaining WasmGroupMember references
rg "WasmGroupMember"
# Search for toSafeGroupMember usage contexts
rg -A 3 "toSafeGroupMember"
Length of output: 973
sdks/browser-sdk/src/workers/client.ts (1)
59-66: LGTM! Verify encryption key handling.
The changes properly integrate the new encryption key requirement and improve logging control with granular levels.
Let's verify the encryption key is consistently required across the codebase:
✅ Verification successful
Verified encryption key handling.
All instances of WorkerClient.create correctly include the encryptionKey, ensuring consistent security across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any direct WorkerClient.create calls that might miss the encryption key
# Search for WorkerClient.create usage
ast-grep --pattern 'WorkerClient.create($$$)'
# Search for createClient usage to ensure encryption key requirement
rg "createClient\(" -A 3
Length of output: 3227
sdks/browser-sdk/src/Client.ts (2)
79-84: Enhancement: Added static create method for streamlined client instantiation
The introduction of the static create method simplifies the process of client instantiation and initialization. This enhancement improves the developer experience by providing a clear and consistent way to create Client instances.
121-133: Feature Addition: Implemented addScwSignature method
The new addScwSignature method adds support for smart contract wallet signatures, aligning with the pull request objectives to enhance SDK functionality in handling smart contracts.
Summary
encryptionKeywhen creating a new clientUnrelated changes
tsconfig.jsonforframes-validatorSummary by CodeRabbit
Release Notes
New Features
@xmtp/wasm-bindingsmodule.Bug Fixes
Documentation
Chores