fix(octicons-styled): update how package is output to reduce bundle size #1027
Conversation
🦋 Changeset detectedLatest commit: 5b7a094 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days. |
|
@joshblack should we consider reviving this PR? Not sure how complete / how close to merge it is. |
|
@lesliecdubs happy to revive this! It should be good to go, just needs a review 👀 One question this does bring up is if we want to continue supporting this "styled-components" package, or not. |
@joshblack do you know how much effort it is to continue supporting this? I can understand that we may not want to commit over the long haul, but if this change will improve performance by only including one version of the icons I'd be interested to ship it and iterate from there. |
|
@lesliecdubs hopefully not much 🤞 And sounds good to me! 👍 |
|
Nice! I'm going to add this PR review to the FR inbox as well so we can hopefully get some movement on that and get closer to merge. |
|
FYI, I'm marking this as ready for review so we can try to get an initial review from a FR on this 😄 |
|
Bump on this, as I saw additional comments on the issue (#981). @joshblack looks like we got a ✅ ; should we merge? |
|
Hey @lesliecdubs! Sorry for the delay, this has been falling behind with the next major work 👀 I noticed an issue with how the bundle gets emitted that I need to check first 🤔 it seems like something key has changed that is causing the bundle to be different than expected |
Closes #981
Update how
@primer/octicons-styledis built so that it doesn't re-include icons in each entrypoint 😅This seems to occur because rollup (at least in the previous version used) does not eliminate unused exports for each of the entry points that were being generated for icons. This PR updates our dependencies on rollup to the latest version and changes how these icons are emitted so that there is only ever one instance of the icons from octicons-react in the bundle.
This also points out that this package could also be importing directly from
@primer/octicons-reactbut I'll save that for another change if we want to go down that route.