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

Enable console feature of web-sys for reactive_graph #3406

Merged
merged 1 commit into from
Dec 24, 2024

Conversation

alexisfontaine
Copy link
Contributor

Without the feature enabled, having a direct dependency to reactive_graph fails to cargo check under wasm32-unknown-unknown.
Just to give you some context, I'm building my own store in a separate library which only needs the reactive primitives.

@alexisfontaine
Copy link
Contributor Author

This seems to be the only occurrence of web_sys in this crate:

web_sys::console::warn_1(&text.to_string().into());

Are there any plans to replace web_sys with a logging library?

@gbj
Copy link
Collaborator

gbj commented Dec 24, 2024

Yeah I mean fair enough

// TODO remove this, it's just useful while developing
#[allow(unused)]
#[doc(hidden)]
pub fn log_warning(text: Arguments) {
#[cfg(feature = "tracing")]
{
tracing::warn!(text);
}
#[cfg(all(
not(feature = "tracing"),
target_arch = "wasm32",
target_os = "unknown"
))]
{
web_sys::console::warn_1(&text.to_string().into());
}

Are there any plans to replace web_sys with a logging library?

If you read the lines above the web_sys part you'll see that there already is a logging library, and that this exists solely to provide a fallback so that logs are not swallowed silently if tracing isn't enabled. (Based on experience, I can say that this is important to avoid a huge amount of user confusion.)

@gbj gbj merged commit 6663fdd into leptos-rs:main Dec 24, 2024
74 checks passed
@alexisfontaine alexisfontaine deleted the fix/reactive-graph-features branch December 25, 2024 16:38
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