Skip to content
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

re-export web_sys for user convenience #568

Closed

Conversation

blainehansen
Copy link
Contributor

I found myself needing to annotate a handler event type, and didn't want to specify my own dependency on web_sys. This pull request should make patterns like the below easy:

use sycamore::web::web_sys;
let handle_enter = |event: web_sys::KeyboardEvent| {
	// ...
};

I also added js_sys. Are there any other libraries we should re-export? Is there any value in re-exporting wasm_bindgen for example?

@codecov
Copy link

codecov bot commented Jan 30, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (9c7bd44) 61.21% compared to head (9ab9345) 61.21%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #568   +/-   ##
=======================================
  Coverage   61.21%   61.21%           
=======================================
  Files          55       55           
  Lines        9021     9021           
=======================================
  Hits         5522     5522           
  Misses       3499     3499           
Files Coverage Δ
packages/sycamore-web/src/lib.rs 20.00% <ø> (ø)

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

@arctic-hen7
Copy link
Contributor

Maybe I'm missing something, but given web-sys is almost entirely controlled by feature flags, wouldn't this mean that users have to import it anyway almost always?

@lukechu10
Copy link
Member

lukechu10 commented Jan 31, 2023

Maybe I'm missing something, but given web-sys is almost entirely controlled by feature flags, wouldn't this mean that users have to import it anyway almost always?

I agree with this sentiment about web-sys. It might make sense to re-export js-sys and wasm-bindgen, although I'm personally a bit iffy on this as well.

Yew used to have a web-sys re-export but they removed it: yewstack/yew#2084
To be fair, they still continue to re-export the event types which they use and so maybe we should be doing something similar?

@Dav1dde
Copy link
Contributor

Dav1dde commented Feb 5, 2023

I think sycamore should only re-export types that are also used in public signatures of sycamore's types and traits.

@lukechu10
Copy link
Member

Hi there, sorry for ignoring this for such a long time. I think what I would like to do is to have sycamore re-export js-sys and wasm-bindgen and have another module events which re-exports all the web-sys events only. Are you still interested in this? If so, please feel free to continue on this PR or open a new PR. If not, please let me know and I can take over. Thanks!

@lukechu10
Copy link
Member

Closed in favor of #642. @blainehansen I've included the original commits from this PR to give you credit.

@lukechu10 lukechu10 closed this Oct 20, 2023
@blainehansen blainehansen deleted the convenience-reexports branch October 23, 2023 20:29
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.

4 participants