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

Fct 1311 - icons bundle 2 #3014

Merged
merged 7 commits into from
Dec 13, 2024
Merged

Fct 1311 - icons bundle 2 #3014

merged 7 commits into from
Dec 13, 2024

Conversation

ByronDWall
Copy link
Contributor

@ByronDWall ByronDWall commented Dec 10, 2024

Updates the SVGR template for generating icons, results in a substantial reduction in bundle size for ui kit icons, as shown in screenshots.

Implements a unique entrypoint for each icon, which enables better tree shaking in consuming applications. This results in a substantial decrease in bundle size for consuming applications, since they can bundle only necessary icons, as shown in the third screenshot.

From 1.35MB to 287kb unzipped, from 341kb to 201kb parsed, from 46kb to 38kb gzipped.

Previous:

Screenshot 2024-12-10 at 1 45 08 PM

Without unique entrypoints:
Screenshot 2024-12-10 at 1 45 05 PM

WITH UNIQUE ENTRYPOINTS FOR EACH ICON:
Screenshot 2024-12-13 at 2 31 30 PM

@ByronDWall ByronDWall self-assigned this Dec 10, 2024
@ByronDWall ByronDWall requested a review from a team as a code owner December 10, 2024 18:52
@ByronDWall ByronDWall requested review from stephsprinkle, jaikamat, ddouglasz, tylermorrisford and misama-ct and removed request for a team December 10, 2024 18:52
Copy link

changeset-bot bot commented Dec 10, 2024

🦋 Changeset detected

Latest commit: 05e1e9c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 98 packages
Name Type
@commercetools-uikit/rich-text-utils Minor
@commercetools-uikit/checkbox-input Minor
@commercetools-uikit/icons Minor
@commercetools-uikit/localized-rich-text-input Minor
@commercetools-uikit/rich-text-input Minor
@commercetools-uikit/select-utils Minor
@commercetools-uikit/inputs Minor
@commercetools-uikit/calendar-utils Minor
@commercetools-uikit/collapsible-panel Minor
@commercetools-uikit/data-table-manager Minor
@commercetools-uikit/data-table Minor
@commercetools-uikit/field-label Minor
@commercetools-uikit/filters Minor
@commercetools-uikit/link Minor
@commercetools-uikit/notifications Minor
@commercetools-uikit/pagination Minor
@commercetools-uikit/primary-action-dropdown Minor
@commercetools-uikit/tag Minor
@commercetools-uikit/password-field Minor
@commercetools-uikit/async-creatable-select-input Minor
@commercetools-uikit/async-select-input Minor
@commercetools-uikit/creatable-select-input Minor
@commercetools-uikit/date-input Minor
@commercetools-uikit/date-range-input Minor
@commercetools-uikit/date-time-input Minor
@commercetools-uikit/input-utils Minor
@commercetools-uikit/localized-money-input Minor
@commercetools-uikit/localized-multiline-text-input Minor
@commercetools-uikit/localized-text-input Minor
@commercetools-uikit/money-input Minor
@commercetools-uikit/multiline-text-input Minor
@commercetools-uikit/radio-input Minor
@commercetools-uikit/search-text-input Minor
@commercetools-uikit/select-input Minor
@commercetools-uikit/selectable-search-input Minor
@commercetools-uikit/time-input Minor
@commercetools-frontend/ui-kit Minor
@commercetools-uikit/search-select-input Minor
@commercetools-uikit/async-creatable-select-field Minor
@commercetools-uikit/async-select-field Minor
@commercetools-uikit/creatable-select-field Minor
@commercetools-uikit/date-field Minor
@commercetools-uikit/date-range-field Minor
@commercetools-uikit/date-time-field Minor
@commercetools-uikit/localized-multiline-text-field Minor
@commercetools-uikit/localized-text-field Minor
@commercetools-uikit/money-field Minor
@commercetools-uikit/multiline-text-field Minor
@commercetools-uikit/number-field Minor
@commercetools-uikit/radio-field Minor
@commercetools-uikit/search-select-field Minor
@commercetools-uikit/select-field Minor
@commercetools-uikit/text-field Minor
@commercetools-uikit/time-field Minor
@commercetools-uikit/quick-filters Minor
@commercetools-uikit/fields Minor
@commercetools-uikit/number-input Minor
@commercetools-uikit/password-input Minor
@commercetools-uikit/text-input Minor
@commercetools-uikit/toggle-input Minor
@commercetools-uikit/design-system Minor
@commercetools-uikit/calendar-time-utils Minor
@commercetools-uikit/hooks Minor
@commercetools-uikit/i18n Minor
@commercetools-uikit/localized-utils Minor
@commercetools-uikit/utils Minor
@commercetools-uikit/accessible-hidden Minor
@commercetools-uikit/avatar Minor
@commercetools-uikit/card Minor
@commercetools-uikit/collapsible-motion Minor
@commercetools-uikit/collapsible Minor
@commercetools-uikit/constraints Minor
@commercetools-uikit/field-errors Minor
@commercetools-uikit/field-warnings Minor
@commercetools-uikit/grid Minor
@commercetools-uikit/label Minor
@commercetools-uikit/loading-spinner Minor
@commercetools-uikit/messages Minor
@commercetools-uikit/progress-bar Minor
@commercetools-uikit/stamp Minor
@commercetools-uikit/text Minor
@commercetools-uikit/tooltip Minor
@commercetools-uikit/view-switcher Minor
@commercetools-uikit/accessible-button Minor
@commercetools-uikit/flat-button Minor
@commercetools-uikit/icon-button Minor
@commercetools-uikit/link-button Minor
@commercetools-uikit/primary-button Minor
@commercetools-uikit/secondary-button Minor
@commercetools-uikit/secondary-icon-button Minor
@commercetools-uikit/dropdown-menu Minor
@commercetools-uikit/spacings-inline Minor
@commercetools-uikit/spacings-inset-squish Minor
@commercetools-uikit/spacings-inset Minor
@commercetools-uikit/spacings-stack Minor
@commercetools-uikit/buttons Minor
@commercetools-uikit/spacings Minor
visual-testing-app Patch

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

Copy link

vercel bot commented Dec 10, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ui-kit ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 13, 2024 9:24pm

@jaikamat
Copy link
Contributor

image
Amazing work 😎

Copy link
Contributor

@CarlosCortizasCT CarlosCortizasCT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! 🎉

Copy link
Contributor

@tdeekens tdeekens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super duper work. Many thanks!

Copy link
Contributor

@tdeekens tdeekens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for going deeper on this! I cant see a diff in SVGR template strangely. By each preconstruct entry we enable the tree shake-ability?

echo "Ignoring PR because of the branch name. Exiting workflow."
exit 1
fi
# - name: Validate branch name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be re-added?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, although it should be updated/fixed in another PR

@@ -0,0 +1,4 @@
{
"main": "dist/commercetools-uikit-icons-generated-ImportReact.cjs.js",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we kebab case the package name part which is the icon?

Copy link
Contributor Author

@ByronDWall ByronDWall Dec 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

everything in packages/icons/generated is auto-generated by preconstruct when you run preconstruct dev or preconstruct build, and there is no way to configure the naming format in preconstruct.

@tdeekens
Copy link
Contributor

Sorry for re-reviewing. Disregard. Just things that appear to the eye when reviewing thrice ;)

@@ -22,10 +22,17 @@
"./index.ts",
"./custom-icon/index.ts",
"./inline-svg/index.ts",
"./leading-icon/index.ts"
"./leading-icon/index.ts",
"./generated/*.tsx"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This glob tells preconstruct that each of the icons should have an entrypoint in the package.

"custom-icon",
"inline-svg",
"leading-icon",
"generated/**"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This glob tells preconstruct that we expect the dist/package.json for each icon to be in the generated folder.

Copy link
Contributor

@tylermorrisford tylermorrisford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice doc!

… remove unnecessary styling boilerplate from icons template
…ls-ui-kit/icons namespace, remove unnecessary pure annotations from icon template in svgr.config.js, add CONTRIBUTING.md file explaining how to add/update/remove icons in ui kit
…add preview branch name check back into preview-release github workflow
…o @commercetools-uikit/icons for backwards compatibility reasons
@ByronDWall ByronDWall force-pushed the FCT-1311-icons-bundle-2 branch from 05e1e9c to 8615362 Compare December 13, 2024 21:21
@ByronDWall ByronDWall merged commit 81e4488 into main Dec 13, 2024
6 checks passed
@ByronDWall ByronDWall deleted the FCT-1311-icons-bundle-2 branch December 13, 2024 21:29
@ct-changesets ct-changesets bot mentioned this pull request Dec 13, 2024
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.

5 participants