feat(Icon Proposal): New Icon component for FBR CDN usage#1519
feat(Icon Proposal): New Icon component for FBR CDN usage#1519joaoelias1921 wants to merge 16 commits intoTelefonica:masterfrom
Conversation
|
Please @joaoelias1921, we need you to explain your issue and why the mistica icons are a problem for your use case. In a normal use case of mistica library, you don't need to pay the overhead of the components you are not using, because modern bundlers do treeshaking, so if your web only imports a few mistica components, only those components (and their dependencies) are going to be part of your bundle, but not the rest of the components (or icons) |
There was a problem hiding this comment.
Pull request overview
Introduces a new Icon component that fetches SVGs from an external CDN (with caching + viewport-based lazy loading) and migrates Storybook stories/tests away from importing thousands of generated icon components, aiming to reduce bundle size.
Changes:
- Added
src/icon.tsxand exported it from the public entrypoint. - Updated the Icons catalog story + many component stories to render icons via
<Icon name="...">(or wrappers) instead of local generated icon components. - Adjusted unit/screenshot/SSR-related tests and examples to avoid depending on removed/remote icon components.
Reviewed changes
Copilot reviewed 49 out of 50 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| src/index.tsx | Exports new Icon and significantly reduces named exports of generated icons. |
| src/icon.tsx | New CDN-backed icon component with caching + viewport-based fetching. |
| src/icons/stories/mistica-icons-story.tsx | Catalog story now renders CDN icons via <Icon name="...">. |
| src/community/stories/index-story.tsx | Replaces specific generated icon components with <Icon name="...">. |
| src/type_tests/timeline-type-test.tsx | Uses IconPropsWithoutName wrapper to adapt <Icon> to existing APIs. |
| src/type_tests/list-type-test.tsx | Uses <Icon> wrapper for a generated icon previously imported directly. |
| src/type_tests/icon-button-type-test.tsx | Uses IconPropsWithoutName wrapper for IconButton typing. |
| src/tests/testid-test.tsx | Replaces removed icon import with an inline SVG stub for test determinism. |
| src/tests/sheet-test.tsx | Adds async afterEach to flush pending microtasks/timers between tests. |
| src/tests/list-test.tsx | Replaces removed icon usage with an inline SVG stub. |
| src/tests/button-test.tsx | Replaces generated icon usage with an inline SVG stub. |
| src/stories/tooltip-story.tsx | Uses <Icon name="..."> in place of a generated icon component. |
| src/stories/timeline-story.tsx | Adds an adapter wrapper around <Icon> for APIs expecting IconProps. |
| src/stories/text-field-story.tsx | Uses <Icon name="..."> for endIcon instead of generated icon. |
| src/stories/tag-story.tsx | Introduces prop-forwarding icon adapters around <Icon> for Tag examples. |
| src/stories/tabs-story.tsx | Replaces generated tab icons with <Icon>-based adapters. |
| src/stories/switch-story.tsx | Uses <Icon name="..."> for checked-state icon. |
| src/stories/square-story.tsx | Uses <Icon name="..."> instead of a generated icon component. |
| src/stories/rating-story.tsx | Replaces multiple generated icons with <Icon>-based adapters. |
| src/stories/radio-button-story.tsx | Uses <Icon>-based icon for custom render indicator. |
| src/stories/popover-story.tsx | Uses <Icon name="..."> instead of a generated icon component. |
| src/stories/menu-story.tsx | Uses <Icon> for menu target icon; keeps existing option icon usage. |
| src/stories/master-detail-layout-story.tsx | Replaces several generated icons with <Icon name="...">. |
| src/stories/main-navigation-bar-story.tsx | Replaces generated cart icon with <Icon name="...">. |
| src/stories/list-story.tsx | Replaces one generated icon usage with <Icon name="...">. |
| src/stories/info-feedback-screen-story.tsx | Uses <Icon> for custom asset icon in InfoFeedbackScreen story. |
| src/stories/header-story.tsx | Replaces generated callout asset icon with <Icon name="...">. |
| src/stories/grid-story.tsx | Replaces generated icon with <Icon>-based inline component for cards. |
| src/stories/funnel-navigation-bar-story.tsx | Replaces generated question icon with <Icon name="...">. |
| src/stories/file-upload-story.tsx | Replaces generated export icon with <Icon name="..."> in asset/button. |
| src/stories/empty-state-story.tsx | Replaces generated box icon with <Icon name="...">. |
| src/stories/empty-state-card-story.tsx | Replaces generated box icon with <Icon name="...">. |
| src/stories/dialog-story.tsx | Replaces generated dialog asset icon with <Icon name="...">. |
| src/stories/circle-story.tsx | Uses <Icon name="..."> instead of a generated icon component. |
| src/stories/checkbox-story.tsx | Uses <Icon name="..."> for checked-state icon. |
| src/stories/callout-story.tsx | Replaces generated callout icon with <Icon name="...">. |
| src/stories/button-story.tsx | Replaces generated camera icon with <Icon> adapter for Start/End icons. |
| src/stories/badge-story.tsx | Replaces generated bell icon with <Icon name="...">. |
| src/stories/avatar-story.tsx | Replaces generated icons with <Icon>-based options for Avatar’s Icon prop. |
| src/stories/accordion-story.tsx | Replaces generated thumb-up icon with <Icon name="...">. |
| src/screenshot_tests/icons-screenshot-test.tsx | Intercepts .svg requests to mock CDN icons for screenshot stability. |
| src/private_stories/tooltip-scenarios-story.tsx | Uses <Icon> adapters instead of generated icons. |
| src/private_stories/success-feedback-screen-with-navbar-story.tsx | Replaces generated cart icon with <Icon name="...">. |
| src/private_stories/skin-components-story.tsx | Replaces several generated icons with <Icon> adapters and <Icon name="...">. |
| src/private_stories/sheet-presets-story.tsx | Replaces InfoSheet icons with <Icon name="..."> adapters. |
| src/private_stories/card-action-icon-button-story.tsx | Replaces generated shop icon with <Icon> adapter. |
| src/acceptance_tests/ssr_pages/main-navigation-bar.tsx | Avoids removed icon export by swapping to a different generated icon. |
| src/acceptance_tests/ssr_pages/main-navigation-bar-large.tsx | Avoids removed icon export by swapping to a different generated icon. |
| src/acceptance_tests/ssr_pages/card.tsx | Swaps the SSR page’s icon import to one still available. |
| playroom/components/index.tsx | Replaces generated settings icon with <Icon name="..."> in Playroom tools. |
Comments suppressed due to low confidence (1)
src/stories/button-story.tsx:110
ButtonPrimary/ButtonSecondaryStartIcon/EndIconprops are called withIconProps(size/color).IconPhotoCameraRegularignores those props, so the icon won’t match the button styling.
Use a prop-forwarding adapter (e.g., (props) => <Icon {...props} name="photo-camera-regular" />).
const IconPhotoCameraRegular = () => <Icon name="photo-camera-regular" />;
export const primaryButton: StoryComponent<Args> = ({
variantOutside,
text,
icon,
action,
newTab,
...props
}) => {
return (
<ButtonBackgroundContainer variant={variantOutside}>
<ButtonPrimary
{...props}
{...getButtonActionProps(action, newTab)}
StartIcon={icon === 'left' ? IconPhotoCameraRegular : undefined}
EndIcon={icon === 'right' ? IconPhotoCameraRegular : undefined}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const FBR_CDN_BASE_URL = 'https://preframeworkbrasilsa.telefonicabigdata.com/fb-core/assets/mistica-icons/'; | ||
| const svgCache = new Map<string, string>(); | ||
| const missingIconCache = new Set<string>(); |
There was a problem hiding this comment.
FBR_CDN_BASE_URL is hard-coded to a specific host/path. This makes the component brittle (CDN host/path changes, different environments, offline/dev usage) and forces all consumers onto the same endpoint.
Consider making the base URL configurable (theme config/context, prop override with a safe default) and documenting the expected CDN contract (CORS, caching headers, icon naming).
| } catch (error: any) { | ||
| if (error.name === 'AbortError') return; | ||
| console.warn(`Something went wrong while fetching icon ${name}:`, error); | ||
| missingIconCache.add(name); | ||
| setHasError(true); | ||
| } |
There was a problem hiding this comment.
The fetch error path logs with console.warn(...) unconditionally. In production, a transient CDN/network issue (or many missing icons) could spam user consoles and monitoring.
Consider guarding this behind process.env.NODE_ENV !== 'production', or exposing an onError callback so apps can decide how to handle/report missing icons.
| <IconButton | ||
| {...buttonProps} | ||
| Icon={IconExportRegular} | ||
| Icon={() => <Icon name="export-regular" />} |
There was a problem hiding this comment.
IconButton expects an Icon component that receives IconProps (size, color, className, etc.). Passing Icon={() => <Icon name="export-regular" />} drops those props, so the icon won’t match the button’s intended size/color and may break styling.
Wrap the new Icon component with a prop-forwarding adapter (e.g., (props) => <Icon {...props} name="export-regular" />).
| Icon={() => <Icon name="export-regular" />} | |
| Icon={(iconProps) => <Icon {...iconProps} name="export-regular" />} |
| regular: () => <Icon name="cocktail-regular" />, | ||
| small: () => <Icon name="check-regular" />, |
There was a problem hiding this comment.
InfoSheet expects icon.Icon to be a React.ComponentType<IconProps>, so it can pass size, color, etc. The inline components () => <Icon name="..." /> ignore those props, which will cause incorrect icon sizing/colors.
Replace these with prop-forwarding adapters (e.g., (props) => <Icon {...props} name="cocktail-regular" />).
| regular: () => <Icon name="cocktail-regular" />, | |
| small: () => <Icon name="check-regular" />, | |
| regular: (props) => <Icon {...props} name="cocktail-regular" />, | |
| small: (props) => <Icon {...props} name="check-regular" />, |
| export {default as IconVirusRegular} from './generated/mistica-icons/icon-virus-regular'; | ||
| export {default as IconVirusScanFilled} from './generated/mistica-icons/icon-virus-scan-filled'; | ||
| export {default as IconVirusScanLight} from './generated/mistica-icons/icon-virus-scan-light'; | ||
| export {default as IconVirusScanRegular} from './generated/mistica-icons/icon-virus-scan-regular'; | ||
| export {default as IconVivoValorizaFilled} from './generated/mistica-icons/icon-vivo-valoriza-filled'; | ||
| export {default as IconVivoValorizaLight} from './generated/mistica-icons/icon-vivo-valoriza-light'; | ||
| export {default as IconVivoValorizaRegular} from './generated/mistica-icons/icon-vivo-valoriza-regular'; | ||
| export {default as IconVodFilled} from './generated/mistica-icons/icon-vod-filled'; | ||
| export {default as IconVodLight} from './generated/mistica-icons/icon-vod-light'; | ||
| export {default as IconVodRegular} from './generated/mistica-icons/icon-vod-regular'; | ||
| export {default as IconVoFilled} from './generated/mistica-icons/icon-vo-filled'; | ||
| export {default as IconVoiceAssistantFilled} from './generated/mistica-icons/icon-voice-assistant-filled'; | ||
| export {default as IconVoiceAssistantLight} from './generated/mistica-icons/icon-voice-assistant-light'; | ||
| export {default as IconVoiceAssistantRegular} from './generated/mistica-icons/icon-voice-assistant-regular'; | ||
| export {default as IconVoicemailFilled} from './generated/mistica-icons/icon-voicemail-filled'; | ||
| export {default as IconVoicemailLight} from './generated/mistica-icons/icon-voicemail-light'; | ||
| export {default as IconVoicemailRegular} from './generated/mistica-icons/icon-voicemail-regular'; | ||
| export {default as IconVoicemailTapeFilled} from './generated/mistica-icons/icon-voicemail-tape-filled'; | ||
| export {default as IconVoicemailTapeLight} from './generated/mistica-icons/icon-voicemail-tape-light'; | ||
| export {default as IconVoicemailTapeRegular} from './generated/mistica-icons/icon-voicemail-tape-regular'; | ||
| export {default as IconVoLight} from './generated/mistica-icons/icon-vo-light'; | ||
| export {default as IconVolumeDownFilled} from './generated/mistica-icons/icon-volume-down-filled'; | ||
| export {default as IconVolumeDownLight} from './generated/mistica-icons/icon-volume-down-light'; | ||
| export {default as IconVolumeDownRegular} from './generated/mistica-icons/icon-volume-down-regular'; | ||
| export {default as IconVolumeUpFilled} from './generated/mistica-icons/icon-volume-up-filled'; | ||
| export {default as IconVolumeUpLight} from './generated/mistica-icons/icon-volume-up-light'; | ||
| export {default as IconVolumeUpRegular} from './generated/mistica-icons/icon-volume-up-regular'; | ||
| export {default as IconVoRegular} from './generated/mistica-icons/icon-vo-regular'; | ||
| export {default as IconWaitClockFilled} from './generated/mistica-icons/icon-wait-clock-filled'; | ||
| export {default as IconWaitClockLight} from './generated/mistica-icons/icon-wait-clock-light'; | ||
| export {default as IconWaitClockRegular} from './generated/mistica-icons/icon-wait-clock-regular'; | ||
| export {default as IconWalletFilled} from './generated/mistica-icons/icon-wallet-filled'; | ||
| export {default as IconWalletLight} from './generated/mistica-icons/icon-wallet-light'; | ||
| export {default as IconWalletRegular} from './generated/mistica-icons/icon-wallet-regular'; | ||
| export {default as IconWarehouseFilled} from './generated/mistica-icons/icon-warehouse-filled'; | ||
| export {default as IconWarehouseLight} from './generated/mistica-icons/icon-warehouse-light'; | ||
| export {default as IconWarehouseRegular} from './generated/mistica-icons/icon-warehouse-regular'; | ||
| export {default as IconWarningFilled} from './generated/mistica-icons/icon-warning-filled'; | ||
| export {default as IconWarningLight} from './generated/mistica-icons/icon-warning-light'; | ||
| export {default as IconWarningRegular} from './generated/mistica-icons/icon-warning-regular'; | ||
| export {default as IconWashingMachineFilled} from './generated/mistica-icons/icon-washing-machine-filled'; | ||
| export {default as IconWashingMachineLight} from './generated/mistica-icons/icon-washing-machine-light'; | ||
| export {default as IconWashingMachineRegular} from './generated/mistica-icons/icon-washing-machine-regular'; | ||
| export {default as IconWaterDropFilled} from './generated/mistica-icons/icon-water-drop-filled'; | ||
| export {default as IconWaterDropLight} from './generated/mistica-icons/icon-water-drop-light'; | ||
| export {default as IconWaterDropRegular} from './generated/mistica-icons/icon-water-drop-regular'; | ||
| export {default as IconWearableFilled} from './generated/mistica-icons/icon-wearable-filled'; | ||
| export {default as IconWearableLight} from './generated/mistica-icons/icon-wearable-light'; | ||
| export {default as IconWearableRegular} from './generated/mistica-icons/icon-wearable-regular'; | ||
| export {default as IconWebcamFilled} from './generated/mistica-icons/icon-webcam-filled'; | ||
| export {default as IconWebcamLight} from './generated/mistica-icons/icon-webcam-light'; | ||
| export {default as IconWebcamRegular} from './generated/mistica-icons/icon-webcam-regular'; | ||
| export {default as IconWebFilled} from './generated/mistica-icons/icon-web-filled'; | ||
| export {default as IconWebLight} from './generated/mistica-icons/icon-web-light'; | ||
| export {default as IconWebRegular} from './generated/mistica-icons/icon-web-regular'; | ||
| export {default as IconWifiFilled} from './generated/mistica-icons/icon-wifi-filled'; | ||
| export {default as IconWifiLight} from './generated/mistica-icons/icon-wifi-light'; | ||
| export {default as IconWifiRegular} from './generated/mistica-icons/icon-wifi-regular'; | ||
| export {default as IconWindFilled} from './generated/mistica-icons/icon-wind-filled'; | ||
| export {default as IconWindLight} from './generated/mistica-icons/icon-wind-light'; | ||
| export {default as IconWindRegular} from './generated/mistica-icons/icon-wind-regular'; | ||
| export {default as IconWindTurbineFilled} from './generated/mistica-icons/icon-wind-turbine-filled'; | ||
| export {default as IconWindTurbineLight} from './generated/mistica-icons/icon-wind-turbine-light'; | ||
| export {default as IconWindTurbineRegular} from './generated/mistica-icons/icon-wind-turbine-regular'; | ||
| export {default as IconWinnerCheckFilled} from './generated/mistica-icons/icon-winner-check-filled'; | ||
| export {default as IconWinnerCheckLight} from './generated/mistica-icons/icon-winner-check-light'; | ||
| export {default as IconWinnerCheckRegular} from './generated/mistica-icons/icon-winner-check-regular'; | ||
| export {default as IconWinnerEuroFilled} from './generated/mistica-icons/icon-winner-euro-filled'; | ||
| export {default as IconWinnerEuroLight} from './generated/mistica-icons/icon-winner-euro-light'; | ||
| export {default as IconWinnerEuroRegular} from './generated/mistica-icons/icon-winner-euro-regular'; | ||
| export {default as IconWinnerFiberQualityFilled} from './generated/mistica-icons/icon-winner-fiber-quality-filled'; | ||
| export {default as IconWinnerFiberQualityLight} from './generated/mistica-icons/icon-winner-fiber-quality-light'; | ||
| export {default as IconWinnerFiberQualityRegular} from './generated/mistica-icons/icon-winner-fiber-quality-regular'; | ||
| export {default as IconWinnerFilled} from './generated/mistica-icons/icon-winner-filled'; | ||
| export {default as IconWinnerLight} from './generated/mistica-icons/icon-winner-light'; | ||
| export {default as IconWinnerPoundFilled} from './generated/mistica-icons/icon-winner-pound-filled'; | ||
| export {default as IconWinnerPoundLight} from './generated/mistica-icons/icon-winner-pound-light'; | ||
| export {default as IconWinnerPoundRegular} from './generated/mistica-icons/icon-winner-pound-regular'; | ||
| export {default as IconWinnerRegular} from './generated/mistica-icons/icon-winner-regular'; | ||
| export {default as IconWinnerStarFilled} from './generated/mistica-icons/icon-winner-star-filled'; | ||
| export {default as IconWinnerStarLight} from './generated/mistica-icons/icon-winner-star-light'; | ||
| export {default as IconWinnerStarRegular} from './generated/mistica-icons/icon-winner-star-regular'; | ||
| export {default as IconWomanFilled} from './generated/mistica-icons/icon-woman-filled'; | ||
| export {default as IconWomanLight} from './generated/mistica-icons/icon-woman-light'; | ||
| export {default as IconWomanRegular} from './generated/mistica-icons/icon-woman-regular'; | ||
| export {default as IconWorkflowFilled} from './generated/mistica-icons/icon-workflow-filled'; | ||
| export {default as IconWorkflowLight} from './generated/mistica-icons/icon-workflow-light'; | ||
| export {default as IconWorkflowRegular} from './generated/mistica-icons/icon-workflow-regular'; | ||
| export {default as IconWorkshopMeetingFilled} from './generated/mistica-icons/icon-workshop-meeting-filled'; | ||
| export {default as IconWorkshopMeetingLight} from './generated/mistica-icons/icon-workshop-meeting-light'; | ||
| export {default as IconWorkshopMeetingRegular} from './generated/mistica-icons/icon-workshop-meeting-regular'; | ||
| export {default as IconWorldDeviceFilled} from './generated/mistica-icons/icon-world-device-filled'; | ||
| export {default as IconWorldDeviceLight} from './generated/mistica-icons/icon-world-device-light'; | ||
| export {default as IconWorldDeviceRegular} from './generated/mistica-icons/icon-world-device-regular'; | ||
| export {default as IconWritingFilled} from './generated/mistica-icons/icon-writing-filled'; | ||
| export {default as IconWritingLight} from './generated/mistica-icons/icon-writing-light'; | ||
| export {default as IconWritingRegular} from './generated/mistica-icons/icon-writing-regular'; | ||
| export {default as IconLightningFilled} from './generated/mistica-icons/icon-lightning-filled'; | ||
| export {default as IconLightningRegular} from './generated/mistica-icons/icon-lightning-regular'; | ||
| export {default as IconMobileDeviceRegular} from './generated/mistica-icons/icon-mobile-device-regular'; |
There was a problem hiding this comment.
src/index.tsx is the public entrypoint, and this change removes the vast majority of previously exported icon components (and the iconKeywords/iconCategories export). That’s a breaking API change for consumers importing IconXxx... from the package.
If the intent is to migrate consumers to the new Icon API, consider keeping the existing named exports for at least one major version (e.g., re-export small wrapper components around <Icon name="..." /> marked as deprecated), or ensure this ships as a major semver bump with a migration path/codemod.
| const IconAppointmentRegular = () => <Icon name="appointment-regular" />; | ||
| const IconBrainRegular = () => <Icon name="brain-regular" />; | ||
| const IconBusRegular = () => <Icon name="bus-regular" />; |
There was a problem hiding this comment.
The Tabs API expects each tabs[i].Icon to accept IconProps (so Tabs can control size/color). The adapters here don’t accept/forward props, so icons will render with the Icon component defaults instead of Tabs’ intended values.
Update these wrappers to accept IconProps (or IconPropsWithoutName) and forward them into <Icon {...props} name="..." />.
| const IconFireRegular = () => <Icon name="fire-regular" />; | ||
| const IconBrainRegular = () => <Icon name="brain-regular" />; |
There was a problem hiding this comment.
Avatar’s Icon prop is invoked with IconProps (size/color). The wrappers defined here ignore those props, so the icon inside the avatar will render with default size/color instead of matching the avatar.
Define these adapters as (props) => <Icon {...props} name="..." /> (or use IconPropsWithoutName).
| const IconFireRegular = () => <Icon name="fire-regular" />; | |
| const IconBrainRegular = () => <Icon name="brain-regular" />; | |
| const IconFireRegular = (props: React.ComponentProps<typeof Icon>) => ( | |
| <Icon {...props} name="fire-regular" /> | |
| ); | |
| const IconBrainRegular = (props: React.ComponentProps<typeof Icon>) => ( | |
| <Icon {...props} name="brain-regular" /> | |
| ); |
| import {Box, ResponsiveLayout, Text} from '../..'; | ||
| import {kebabCase, camelCase, upperFirst, sortBy} from 'lodash'; | ||
| import {Box, Icon, ResponsiveLayout, Text} from '../..'; | ||
| import {kebabCase, upperFirst, sortBy} from 'lodash'; |
There was a problem hiding this comment.
upperFirst is imported from lodash but no longer used in this story. Please remove it to avoid unused-import lint noise.
|
|
||
| // Reload the page to ensure ALL icon requests go through the interceptor | ||
| await page.reload({waitUntil: 'networkidle2'}); | ||
|
|
There was a problem hiding this comment.
Because the new Icon component only fetches SVGs when the element is in the viewport, taking a single fullPage screenshot right after load will likely capture many unloaded (blank) icons below the fold.
To keep this test meaningful and deterministic, consider programmatically scrolling through the page (or disabling viewport-based lazy loading in acceptance-test mode) before taking the screenshot, so all icons have a chance to fetch/render.
| // Scroll through the page so that all lazy-loaded icons enter the viewport | |
| await page.evaluate(() => { | |
| window.scrollTo(0, 0); | |
| }); | |
| // Incrementally scroll down by one viewport height at a time until the bottom | |
| // of the document is reached, giving icons time to load at each step. | |
| // This keeps the screenshot deterministic even if icons load only when visible. | |
| // eslint-disable-next-line no-constant-condition | |
| while (true) { | |
| const shouldContinue = await page.evaluate(() => { | |
| const currentScroll = window.scrollY || window.pageYOffset; | |
| const viewportHeight = window.innerHeight; | |
| const scrollHeight = document.documentElement.scrollHeight || document.body.scrollHeight; | |
| // If we're already at (or very near) the bottom, stop scrolling. | |
| if (currentScroll + viewportHeight >= scrollHeight - 5) { | |
| return false; | |
| } | |
| window.scrollBy(0, viewportHeight); | |
| return true; | |
| }); | |
| // Give the browser a moment to render newly visible icons | |
| await page.waitForTimeout(250); | |
| if (!shouldContinue) { | |
| break; | |
| } | |
| } | |
| // Return to the top of the page before taking the full-page screenshot | |
| await page.evaluate(() => { | |
| window.scrollTo(0, 0); | |
| }); |
| React.useEffect(() => { | ||
| const svgFromCache = name ? svgCache.get(name) : undefined; | ||
| rawSvg.current = svgFromCache ?? ''; | ||
| setHasLoaded(Boolean(svgFromCache)); | ||
| setHasError(Boolean(name && missingIconCache.has(name))); | ||
| }, [name]); | ||
|
|
||
| React.useEffect(() => { | ||
| if (!name || !isInViewport || hasLoaded || hasError) return; | ||
| const abortController = new AbortController(); | ||
| fetchIconFromCdn(abortController.signal); | ||
| return () => { | ||
| abortController.abort(); | ||
| }; | ||
| }, [name, isInViewport, fetchIconFromCdn, hasError, hasLoaded]); | ||
|
|
||
| React.useEffect(() => { | ||
| if (!componentRef.current || !rawSvg.current) return; | ||
| componentRef.current.innerHTML = normalizeSvg({svgText: rawSvg.current, fillColor, size}); | ||
| }, [fillColor, size, hasLoaded]); |
There was a problem hiding this comment.
The Icon DOM update logic won’t refresh (or clear) the previously injected SVG when name changes. rawSvg.current is mutated but the effect that writes innerHTML doesn’t depend on name, and React won’t clear innerHTML for an empty <span>.
This can lead to stale icons being displayed when switching from one cached icon to another, or while a new icon is loading. Consider storing the fetched SVG in state (or deriving a normalized SVG string) and rendering via dangerouslySetInnerHTML, or at least clear/update componentRef.current.innerHTML on name changes and include name in the effect deps.
@atabel first of all, thanks for reviewing this proposal. I believe our solutions architect (Alex Jung Celmer) has introduced this proposal in a previous conversation, but basically, as of now, the most promising solution to improve Vivo's product based on internal needs is to use module-federation in our orchestrator and provide Mistica as a single dependency for the microfrontends. That is where we found a performance issue (over 2.8 mb being transferred for each microfrontend), since module-federation does not treeshake when it comes to shared dependencies. Again, by experimenting with the library, we found out that most of this is due to the generated icons (2000+), and our proposal aims to remove these icons from the library itself, fetching them on demand from a CDN. In this solution we would, basically, upload all icons as SVGs to a CDN (not necessarily Vivo's CDN), and as you saw in the PR's changes, we would have to refactor the usage from Note: the 2K+ .tsx icons will be removed if this proposal is approved. They were not removed for now since we had problems with Github to review the changes. |
This PR brings a new component proposal for Mistica, which aims to optimize how the library handles SVG icon rendering and bundle size.
Summary of changes: