8000 Make `main` optional when compiling to WASM by barafael · Pull Request #11 · twitchax/kord · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Make main optional when compiling to WASM #11

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

Merged
merged 2 commits into from
Jun 6, 2023
Merged

Make main optional when compiling to WASM #11

merged 2 commits into from
Jun 6, 2023

Conversation

barafael
Copy link
Contributor
@barafael barafael commented Jun 1, 2023

Not sure this is idiomatic. However, I noticed, that when compiling to e.g. a yew app, which has a main, it conflicts with the existing main from kord. The double negation of not("wasm_no_main") is because I didn't want to make this a breaking change, which I guess a feature "wasm_main" would have been. Let me know if you prefer a feature "wasm_main" as part of the default-features perhaps, or something else entirely.

Not sure this is idiomatic. However, I noticed, that when compiling to e.g. a yew app, which has a `main`, it conflicts with the existing `main` from `kord`.
The double negation of `not("wasm_no_main")` is because I didn't want to make this a breaking change, which I guess a feature `"wasm_main"` would have been.
Let me know if you prefer a feature `"wasm_main"` as part of the `default-features` perhaps, or something else entirely.
@twitchax
Copy link
Owner
twitchax commented Jun 2, 2023

Actually, I think the right thing to do idiomatically is to delete the main entirely. It probably shouldn't have a main, except for debugging, so maybe that's the best way to cfg it away.

@barafael
Copy link
Contributor Author
barafael commented Jun 2, 2023

Here's a variant without a main in WASM at all. One could add it for debugging. I have kept set_panic_hook in, however I'd also argue it is up for removal. A library user would do it themselves, so certainly a pub function seems out of place.

Maybe a good compromise would be to simply keep the original fn main but comment it out. No features and no set_panic_hook. This would be a breaking change, however.

What do you think?

@twitchax
Copy link
Owner
twitchax commented Jun 2, 2023

Yeah, I think we should remove them altogether.

@twitchax twitchax merged commit fea593f into twitchax:main Jun 6, 2023
@barafael barafael deleted the wasm-main-feature branch June 6, 2023 08:51
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
0