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

Feature gate icons #46

Open
Stefanuk12 opened this issue Feb 22, 2024 · 7 comments
Open

Feature gate icons #46

Stefanuk12 opened this issue Feb 22, 2024 · 7 comments

Comments

@Stefanuk12
Copy link

As you have acknowledged, having so many icons causes rust-analyzer to lag. To fix this, you can gate each icon into its own feature flag. If you reach over the maximum features, you can group icons instead and do it that way.

@carloskiki
Copy link
Owner

Yes, this crate is the cause which instigated limits on crate features: https://blog.rust-lang.org/2023/10/26/broken-badges-and-23k-keywords.html.

The only obvious way I see of partitioning icons with feature flags would be on a per-library basis. This feels somewhat of a coarse grain method, but I could get behind it as it will improve UX.

If you have time, I would encourage you to open a PR and try your hand at implementing per-library features. I would be happy to review it :), but I do not have the time to implement it myself.

@martijnarts
Copy link

This would have to happen on the https://github.com/carloskiki/icondata/ repo, right?

@carloskiki
Copy link
Owner

Yes exactly!

For a bit of context, what we did before is that we feature gated every single icon individually. This created a lot of needless churn for the compiler and Rust's infrastructure. I refer you to this blog post from the official Rust team which describes the issue in detail.

Following this, we decided not to use any feature flags, the only downside being that now rust-analyzer is under a lot of pressure from our crate (due to the massive amount of statics we make available).

I think a nice granularity to have would be icon-lib-wise; i.e., Font Awesome, Octicons, etc., would each have a feature flag. This would leave us with 20 or so feature flags. This would alleviate some of the work for RA, and would not be to painful for the end user.
What do you think?

@martijnarts
Copy link

I'd question what the purpose is of feature flagging. If it's only to reduce strain on RA, I think doing one flag per icon lib is perfectly fine!

I don't think having more icons increases something like the final binary size, right, since they'd just get compiled out? So then having per-icon flags doesn't really do much, I suppose?

@martijnarts
Copy link

I've opened a PR for the feature flags here: carloskiki/icondata#39 :)

@axos88
Copy link

axos88 commented Nov 1, 2024

Are unused icons optimized away from the generated wasm binary? Or is the entire icon set written to the final binary? I'm not entirely sure whether that's automatic, or something needs to be done? Obviously wouldn't want 23k icons to bloat my binary.

@carloskiki
Copy link
Owner

Are unused icons optimized away from the generated wasm binary? Or is the entire icon set written to the final binary? I'm not entirely sure whether that's automatic, or something needs to be done? Obviously wouldn't want 23k icons to bloat my binary.

No, by personal experience all unused icons are removed by Dead Code Elimination. I've also never seen anyone show that icons were kept, although we are indeed relying on the compiler to do the work for us. You can (should) always inspect your wasm bundle with twiggy.

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

No branches or pull requests

4 participants