fix: handling buffer with node or browser#60
Conversation
There was a problem hiding this comment.
Code Review
This pull request removes the global Node.js type dependency by refactoring the getOrgIdFromPublicKey method to use globalThis.atob when available, falling back to a dynamically resolved Buffer. A review comment suggests a more robust implementation for the Buffer fallback to prevent runtime errors in environments where Buffer might be present but missing the from method.
| const nodeBuffer = ( | ||
| globalThis as typeof globalThis & { | ||
| Buffer?: { | ||
| from(input: string, encoding: string): { toString(): string }; | ||
| }; | ||
| } | ||
| ).Buffer; | ||
| const decoded = globalThis.atob | ||
| ? globalThis.atob(keyWithoutPrefix) | ||
| : Buffer.from(keyWithoutPrefix, "base64").toString(); | ||
| : (nodeBuffer?.from(keyWithoutPrefix, "base64").toString() ?? ""); |
There was a problem hiding this comment.
The current implementation for detecting and using Buffer is quite verbose and potentially unsafe. If globalThis.Buffer exists but does not have a from method (e.g., in certain polyfilled environments or very old Node.js versions), nodeBuffer?.from(...) will attempt to call undefined as a function, leading to a runtime error. A more robust and concise approach would be to use optional chaining on the method call itself and check the type of atob more strictly.
const decoded = typeof globalThis.atob === "function"
? globalThis.atob(keyWithoutPrefix)
: (globalThis as any).Buffer?.from?.(keyWithoutPrefix, "base64")?.toString() ?? "";
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
fix: handling buffer with node or browser