Skip to content

Conversation

@demhadais
Copy link

This pull request adds the conversion impls for wasm-bindgen crate's traits to enable usage in structs annotated with #[wasm_bindgen]

@codecov
Copy link

codecov bot commented Oct 7, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.90%. Comparing base (ebc808e) to head (9949fed).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1750      +/-   ##
==========================================
- Coverage   90.90%   90.90%   -0.01%     
==========================================
  Files          39       39              
  Lines       16254    16254              
==========================================
- Hits        14776    14775       -1     
- Misses       1478     1479       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@demhadais
Copy link
Author

Closes #1749

@demhadais
Copy link
Author

Per the recommendations on BurntSushi/jiff#424, I just want to alert the maintainers of chrono that the module these traits live in (wasm-bindgen::convert) is unstable and internal. Implementing these traits could pose semver compatibility issues.

}
}

#[cfg(all(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make this verbose, suggest wrapping these in a module, like so:

#[cfg(all(
    target_arch = "wasm32",
    feature = "wasmbind",
    not(any(target_os = "emscripten", target_os = "wasi", target_os = "linux"))
))]
mod wasm_impls {
    use wasm_bindgen::convert::{FromWasmAbi, ..};

    impl .. { }

    // etc
}

@djc
Copy link
Member

djc commented Oct 9, 2025

(Please squash any changes from feedback into the original commit.)

move wasm-bindgen trait impls into one module to reduce verbosity

add datetime and utc imports to wasm-impls module as they were erroneously left out of the last commit
Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@demhadais
Copy link
Author

@djc done! Thanks for the feedback. Again, I'd just like to alert you that the traits I've implemented are specifically marked as unstable, which poses semver issues. In practice, I'm not sure how much of an actual issue it will be, but I figured I'd reiterate in case you didn't see it last time.

@djc
Copy link
Member

djc commented Oct 14, 2025

Ah, sorry, I missed your last comment on that. I think I'm still willing to take this, although I want an extra opt-in Cargo feature (let's call it wasm-bindgen-unstable) and will basically ignore any breakage from this.

@demhadais
Copy link
Author

Sounds good to me! This will be incredibly useful for me so I'm happy this is going through.

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.

2 participants