Skip to content

Conversation

@insipx
Copy link
Contributor

@insipx insipx commented Dec 1, 2025

bindings_wasm decodeText clashes with another declared function. I'm not sure if it was a function declared in bindgen or just the test runner, but changing decodeText to decodeXmtpText fixed it. In our code, i only found a single decodeText. I changed encodeText to encodeXmtpText for consistency

Error message ci/local tests failing with:

Uncaught SyntaxError: redeclaration of function decodeText

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

function getUint8ArrayMemory0() {
    if (cachedUint8ArrayMemory0 === null || cachedUint8ArrayMemory0.byteLength === 0) {
        cachedUint8ArrayMemory0 = new Uint8Array(wasm.memory.buffer);
    }
    return cachedUint8ArrayMemory0;
}

let cachedTextDecoder = new TextDecoder('utf-8', { ignoreBOM: true, fatal: true });

cachedTextDecoder.decode();

const MAX_SAFARI_DECODE_BYTES = 2146435072;
let numBytesDecoded = 0;
function decodeText(ptr, len) {
    numBytesDecoded += len;
    if (numBytesDecoded >= MAX_SAFARI_DECODE_BYTES) {
        cachedTextDecoder = new TextDecoder('utf-8', { ignoreBOM: true, fatal: true });
        cachedTextDecoder.decode();
        numBytesDecoded = len;
    }
    return cachedTextDecoder.decode(getUint8ArrayMemory0().subarray(ptr, ptr + len));
}

function getStringFromWasm0(ptr, len) {
    ptr = ptr >>> 0;
    return decodeText(ptr, len);
}

@insipx insipx requested a review from rygine December 1, 2025 14:28
@insipx insipx requested a review from a team as a code owner December 1, 2025 14:28
@graphite-app
Copy link

graphite-app bot commented Dec 1, 2025

How to use the Graphite Merge Queue

Add 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
Copy link

claude bot commented Dec 1, 2025

Claude finished @insipx's task —— View job


Code Review - December 1, 2025, 14:47 UTC

  • Read and analyze the changed file
  • Check for test coverage implications
  • Provide review feedback

The fix is correct and necessary. The renaming from decodeText/encodeText to decodeXmtpText/encodeXmtpText properly resolves the name collision with wasm-bindgen's internal decodeText function.

Key Points:

✓ Code Quality: Clean implementation with appropriate error handling. The js_name annotation correctly exports the Rust functions with the new names.

✓ Naming Convention: The Xmtp prefix clearly distinguishes these from generic text encoding utilities and follows a reasonable pattern for avoiding conflicts in the global WASM namespace.

⚠️ Documentation Gap: Consider adding a brief comment in bindings_wasm/src/content_types/text.rs:22 explaining why the Xmtp prefix is used (similar to the comment on line 36-38 for decode), as this would help future maintainers understand the naming choice for both functions.

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.


@macroscopeapp
Copy link
Contributor

macroscopeapp bot commented Dec 1, 2025

Rename WebAssembly exports in bindings_wasm/src/content_types/text.rs to fix build by changing wasm_bindgen js_name for encode_text to encodeXmtpText and decode_text to decodeXmtpText

Updates #[wasm_bindgen(js_name = ...)] on encode_text and decode_text to encodeXmtpText and decodeXmtpText in bindings_wasm/src/content_types/text.rs. Adds comments noting a name conflict.

📍Where to Start

Start with the #[wasm_bindgen] exports for encode_text and decode_text in bindings_wasm/src/content_types/text.rs.


Macroscope summarized 26a9266.

@insipx insipx enabled auto-merge (squash) December 1, 2025 14:29
@insipx insipx force-pushed the push-tkyvzsqvnkyz branch 2 times, most recently from dbdde88 to e00de8f Compare December 1, 2025 14:30
@insipx insipx force-pushed the push-tkyvzsqvnkyz branch from e00de8f to 26a9266 Compare December 1, 2025 14:46
@insipx
Copy link
Contributor Author

insipx commented Dec 1, 2025

@insipx insipx changed the title fix webassembly bindings_wasm build fix webassembly bindings_wasm tests Dec 1, 2025
@insipx insipx changed the title fix webassembly bindings_wasm tests fix webassembly bindings_wasm decodeText clash Dec 1, 2025
@insipx insipx changed the title fix webassembly bindings_wasm decodeText clash fix webassembly test bindings_wasm decodeText clash Dec 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants