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

[Feedback] npm package contains duplicate icon data #699

Closed
xt0rted opened this issue Nov 20, 2021 · 16 comments
Closed

[Feedback] npm package contains duplicate icon data #699

xt0rted opened this issue Nov 20, 2021 · 16 comments

Comments

@xt0rted
Copy link
Contributor

xt0rted commented Nov 20, 2021

Describe the topic

I just installed v16.1.1 of @primer/octicons and noticed the svg folder and data.json file are duplicated in the package. I don't think this is intended and it's doubling the size of the package.

image

@ashygee
Copy link
Contributor

ashygee commented Nov 22, 2021

Hi @xt0rted, thanks for letting us know! We'll take a look at this once we get some open bandwidth.

@ashygee ashygee assigned jonrohan and colebemis and unassigned colebemis Nov 22, 2021
@github-actions
Copy link
Contributor

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

@github-actions github-actions bot added the Stale label May 21, 2022
@xt0rted
Copy link
Contributor Author

xt0rted commented May 21, 2022

Still an issue in 17.2.0

@lesliecdubs
Copy link
Member

@jonrohan is this something you might be able to squeeze in this quarter?

@tallys
Copy link
Contributor

tallys commented Jan 5, 2023

Following up, @jonrohan is this still an issue or can we close this?

@jonrohan
Copy link
Member

jonrohan commented Jan 5, 2023

I think this is still an issue. I believe both data sources are in use, so we'd need to investigate which one we can remove and update any libraries using them.

@jonrohan jonrohan removed their assignment Jan 31, 2023
@tallys
Copy link
Contributor

tallys commented Jan 31, 2023

Hey @JoshBowdenConcepts, this might be a good one to pick up - thoughts?

@github-actions
Copy link
Contributor

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

@github-actions github-actions bot added the Stale label Jul 30, 2023
@github-actions github-actions bot closed this as completed Aug 6, 2023
@raytalks raytalks reopened this Aug 7, 2023
@tallys
Copy link
Contributor

tallys commented Aug 7, 2023

Thank you for raising this issue, it is a known error and is in our backlog.

@fregante
Copy link
Contributor

Would you accept a PR? Sounds like a quick change. This bug doubles the size of the package

https://packagephobia.com/result?p=@primer/octicons@13.0.0

@lesliecdubs
Copy link
Member

Hi @fregante, thanks for the offer. Yes, we would gladly accept a PR for this change!

@fregante
Copy link
Contributor

@lesliecdubs done

@lesliecdubs
Copy link
Member

Thanks @fregante! ✨ We might be a little slower than usual with PR review due to the holiday season, but we should be able to get to it in the new year. Thanks for contibuting!

@fregante
Copy link
Contributor

fregante commented Mar 13, 2024

This can be closed once #1010 is published to npm. We should see the halving of the package here: https://packagephobia.com/result?p=@primer/octicons

@joshblack
Copy link
Member

Just wanted to confirm that I don't see this anymore in v19.9.0. Going to close this out as I believe it's been addressed, feel free to leave a comment if I'm getting something wrong here!

@fregante
Copy link
Contributor

Indeed in my link you can see the package going from 2.16 to 1.08, exactly half

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants