-
Notifications
You must be signed in to change notification settings - Fork 74
fix webassembly test bindings_wasm decodeText clash
#2863
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
How to use the Graphite Merge QueueAdd the label mergequeue to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
|
Claude finished @insipx's task —— View job Code Review - December 1, 2025, 14:47 UTC
The fix is correct and necessary. The renaming from Key Points:✓ Code Quality: Clean implementation with appropriate error handling. The ✓ Naming Convention: The
Test Coverage: No test files currently reference these functions. Since these were recently added in PR #2807, and this is purely a renaming change that fixes a build error, the lack of updated tests is acceptable for this fix. However, consider adding integration tests that exercise these functions in a future PR to ensure they work correctly in the WASM environment. No security, performance, or bug concerns identified. |
Rename WebAssembly exports in
|
dbdde88 to
e00de8f
Compare
e00de8f to
26a9266
Compare
|
This PR is part of a stack of 9 bookmarks:
Created with jj-stack |
bindings_wasm buildbindings_wasm tests
bindings_wasm testsbindings_wasm decodeText clash
bindings_wasm decodeText clashbindings_wasm decodeText clash
bindings_wasm
decodeTextclashes with another declared function. I'm not sure if it was a function declared in bindgen or just the test runner, but changingdecodeTexttodecodeXmtpTextfixed it. In our code, i only found a singledecodeText. I changedencodeTexttoencodeXmtpTextfor consistencyError message ci/local tests failing with:
it looks like some internal bindgen/js glue code, here is the function its conflicting with for historical purposes (from the sourcemap in console).
mostly i'm surprised these functions are not prefixed with
__to avoid clashes like this