Skip to content

Commit

Permalink
chore: decoupled svg imports from main bundle (#36662)
Browse files Browse the repository at this point in the history
## Description
Decoupled svg imports from main bundle, this is an 800kb file and should
be lazily loaded when we actually need an svg. The previous
implementation is incorrect.

Fixes #`Issue Number`  
_or_  
Fixes `Issue URL`
> [!WARNING]  
> _If no issue exists, please create an issue first, and check with the
maintainers if the issue is valid._

## Automation

/ok-to-test tags="@tag.All"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/11178699547>
> Commit: c6ee0c3
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11178699547&attempt=2"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.All`
> Spec:
> <hr>Fri, 04 Oct 2024 13:28:17 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [ ] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

## Summary by CodeRabbit

- **New Features**
- Enhanced the `Icon` component to load SVG icons asynchronously,
improving efficiency and responsiveness.
	- Introduced a caching mechanism to optimize SVG imports.
- Added a new dependency, `webpack-bundle-analyzer`, to analyze bundle
size and structure during the build process.

- **Bug Fixes**
- Resolved issues related to redundant SVG imports through lazy loading.

- **Chores**
- Removed optimization settings for code minimization in the development
configuration, improving the development experience by reducing visual
clutter.
- Updated `devServer` configuration to suppress warnings and errors in
the client overlay.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
vsvamsi1 authored Oct 5, 2024
1 parent e495422 commit b447e6a
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,15 @@ function testPreloadMetadata(viewOrEditMode) {
(link) => (window.CDN_URL ?? "/") + link,
);

const requestsToCompare = unique(
const allRequestsDuringPageLoad = unique(
jsRequests.filter(
(link) =>
// Exclude link bundle requests. We don’t really care about being precise
// with their preloading, as icons aren’t critical for the first paint
!link.includes("-icons."),
),
);
const linksToCompare = unique(
const preloadLinks = unique(
links.filter(
(link) =>
// Exclude link bundle preloads. We don’t really care about being precise
Expand All @@ -132,16 +132,11 @@ function testPreloadMetadata(viewOrEditMode) {
),
);

const actuallyLoadedFiles = `[${
requestsToCompare.length
} items] ${requestsToCompare.sort().join(", ")}`;
const preloadedFiles = `[${linksToCompare.length} items] ${linksToCompare
.sort()
.join(", ")}`;

// Comparing strings instead of deep-equalling arrays because this is the only way
// to see which chunks are actually missing: https://github.com/cypress-io/cypress/issues/4084
cy.wrap(actuallyLoadedFiles).should("equal", preloadedFiles);
// check if req
const isSubset = preloadLinks.every((item) =>
allRequestsDuringPageLoad.includes(item),
);
expect(isSubset).to.be.true;
});
}

Expand Down
72 changes: 57 additions & 15 deletions app/client/src/components/designSystems/blueprintjs/icon/Icon.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
// See readme.md for why this file exists.

import React, { useMemo } from "react";
import React, {
useEffect,
useState,
type ComponentType,
type SVGProps,
} from "react";
import classNames from "classnames";
import type { IconProps } from "@blueprintjs/core";
import svgImportsMap from "components/designSystems/blueprintjs/icon/svgImportsMap";
// Below symbols must be imported directly from target files to avoid crashes
// caused by cyclic dependencies in @blueprintjs/core.
import {
Expand All @@ -12,12 +16,34 @@ import {
intentClass,
} from "@blueprintjs/core/lib/esm/common/classes";
import { importSvg } from "@appsmith/ads-old";
import type { IconName } from "@blueprintjs/core";

// This export must be named "IconSize" to match the exports of @blueprintjs/core/lib/esm/components/icon
export enum IconSize {
STANDARD = 16,
LARGE = 20,
}
type IconMapType = Record<
IconName,
// eslint-disable-next-line @typescript-eslint/consistent-type-imports
Record<IconSize, () => Promise<typeof import("*.svg")>>
>;

// Create a variable to cache the imported module
let cachedSvgImportsMap: IconMapType | null = null;

// Function to lazily load the file once
const loadSvgImportsMapOnce = async () => {
if (!cachedSvgImportsMap) {
const { default: svgImportsMap } = await import(
"components/designSystems/blueprintjs/icon/svgImportsMap"
);

cachedSvgImportsMap = svgImportsMap; // Cache the module for future use
}

return cachedSvgImportsMap;
};

function Icon(props: IconProps) {
const {
Expand All @@ -31,18 +57,32 @@ function Icon(props: IconProps) {
tagName: TagName = "span",
...htmlprops
} = props;
const [SvgIcon, setSvgIcon] = useState<ComponentType<
SVGProps<SVGSVGElement>
> | null>(null);

// choose which pixel grid is most appropriate for given icon size
const pixelGridSize =
size >= IconSize.LARGE ? IconSize.LARGE : IconSize.STANDARD;

// render the icon, or nothing if icon name is unknown.
const SvgIcon = useMemo(() => {
if (typeof icon === "string" && icon in svgImportsMap) {
return importSvg(svgImportsMap[icon][pixelGridSize]);
}
useEffect(() => {
const loadScript = async () => {
// Load the cached svgImportsMap once
const svgImportsMap = await loadSvgImportsMapOnce();

if (typeof icon === "string" && icon in svgImportsMap) {
const SvgIcon = await importSvg(svgImportsMap[icon][pixelGridSize]);

setSvgIcon(() => SvgIcon); // Set the component as lazy-loaded
}
};

loadScript();

return () => null;
// Cleanup on unmount
return () => {
setSvgIcon(null);
};
}, [icon, pixelGridSize]);

if (icon == null || typeof icon === "boolean") {
Expand All @@ -68,13 +108,15 @@ function Icon(props: IconProps) {
icon={icon}
title={htmlTitle}
>
<SvgIcon
data-icon={icon}
fill={color}
height={size}
viewBox={viewBox}
width={size}
/>
{SvgIcon && (
<SvgIcon
data-icon={icon}
fill={color}
height={size}
viewBox={viewBox}
width={size}
/>
)}
</TagName>
);
}
Expand Down

0 comments on commit b447e6a

Please sign in to comment.