chore: decoupled svg imports from main bundle #36662
Conversation
WalkthroughThe changes introduce a new dependency, Changes
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
the
skip-changelog
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
app/client/src/components/designSystems/blueprintjs/icon/Icon.tsx (2)
83-85: Consider the necessity of state updates during cleanupIn the cleanup function of your
useEffect, you're settingSvgIcontonull. While it's good practice to clean up, setting the state during unmount may not be necessary since the component is being removed.Removing this line can prevent potential memory leaks or unnecessary re-renders. Think about whether this state update is required.
// Cleanup on unmount return () => { - setSvgIcon(null); };
111-119: Provide a fallback UI while the icon is loadingBecause
SvgIconis loaded asynchronously, there may be a brief moment when it'snull. To improve the user interface, consider providing a fallback element or loader during this time.For example, you can display a placeholder icon or a spinner:
{SvgIcon ? ( <SvgIcon data-icon={icon} fill={color} height={size} viewBox={viewBox} width={size} /> ) : ( // Fallback content while the icon loads <DefaultIcon height={size} width={size} /> )}This ensures that the user interface remains consistent and provides feedback while the icon is being fetched.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- app/client/src/components/designSystems/blueprintjs/icon/Icon.tsx (4 hunks)
🔇 Additional comments (1)
app/client/src/components/designSystems/blueprintjs/icon/Icon.tsx (1)
36-46: Excellent use of lazy loading for SVG importsGreat job implementing lazy loading with the
loadSvgImportsMapOncefunction! By caching the SVG imports map, you're optimizing performance and ensuring that the icons are only loaded when needed. Keep up the good work in utilizing efficient coding practices.
| 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); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Let's enhance error handling in asynchronous operations
When dealing with asynchronous functions like loadScript, it's important to anticipate potential errors that might occur during the loading process. To make our code more robust, consider adding a try-catch block to handle any exceptions gracefully.
Here's how you might modify the loadScript function:
useEffect(() => {
const loadScript = async () => {
+ try {
// 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
}
+ } catch (error) {
+ console.error("Error loading SVG icon:", error);
+ // You might set a default icon or handle the error state here
+ setSvgIcon(null);
+ }
};
loadScript();
// Cleanup on unmount
return () => {
setSvgIcon(null);
};
}, [icon, pixelGridSize]);By adding error handling, we ensure that our application remains stable even if an icon fails to load. This will enhance the user experience and prevent unforeseen issues.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 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); | |
| }; | |
| useEffect(() => { | |
| const loadScript = async () => { | |
| try { | |
| // 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 | |
| } | |
| } catch (error) { | |
| console.error("Error loading SVG icon:", error); | |
| // You might set a default icon or handle the error state here | |
| setSvgIcon(null); | |
| } | |
| }; | |
| loadScript(); | |
| // Cleanup on unmount | |
| return () => { | |
| setSvgIcon(null); | |
| }; | |
| }, [icon, pixelGridSize]); |
vsvamsi1
added
ok-to-test
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
app/client/craco.common.config.js (1)
5-5: Class, let's discuss the broader implications of our new addition.Adding the BundleAnalyzerPlugin is like getting a new microscope for our code laboratory. It's exciting, but we need to understand its effects:
- Build time: Our experiments might take a bit longer to set up now.
- Extra data: We'll be generating new reports with each build.
- Bundle size: Don't worry, this won't make our final project any bigger!
For your next assignment, please update our project documentation to include:
- What the BundleAnalyzerPlugin does
- How to read and use the reports it generates
- Any impact on our development workflow
Remember, good scientists always document their new procedures!
Also applies to: 145-145
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
app/client/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
- app/client/craco.common.config.js (2 hunks)
- app/client/craco.dev.config.js (0 hunks)
- app/client/package.json (1 hunks)
- app/client/src/components/designSystems/blueprintjs/icon/Icon.tsx (4 hunks)
💤 Files with no reviewable changes (1)
- app/client/craco.dev.config.js
🔇 Additional comments (7)
app/client/src/components/designSystems/blueprintjs/icon/Icon.tsx (4)
3-8: Class, let's examine these new imports and type definition.Good morning, students! Today, we're going to learn about some important changes in our code. Can you see how we've added new React hooks and types to our import statement? These are like new tools in our coding toolbox. They'll help us build a better, more efficient Icon component.
Now, let's look at this new
IconMapType. It's like a special dictionary that tells our program where to find each icon. This will make our code more organized and easier to understand.Also applies to: 26-30
32-47: Let's discuss our new caching system, class!Alright, students, pay attention to this exciting new feature! We've introduced a caching system. Can anyone tell me what caching means? That's right, it's like remembering something so we don't have to look it up again!
Look at our
cachedSvgImportsMapvariable. It's like a big box where we store our SVG imports after we load them the first time. And ourloadSvgImportsMapOncefunction? It's the smart helper that checks if we've already filled our box before going to get new SVGs.This is a wonderful optimization, class. It means our program will run faster because it doesn't have to keep loading the same things over and over. Isn't that clever?
112-120: Let's examine our new rendering logic, class!Excellent work here, students! Do you see how we're using a conditional statement to render our SvgIcon? This is a very important concept in React.
By writing
{SvgIcon && (...)}, we're saying, "Only show this icon if SvgIcon exists." It's like checking if we have all our ingredients before we start cooking. This prevents errors and makes our code more robust.Remember, class: always check if something exists before you try to use it in your code. It's a good habit that will save you from many headaches in the future!
Line range hint
1-126: Class, let's recap what we've learned today!Well done, everyone! We've gone through some significant changes in our Icon component. Let's summarize what we've learned:
- We've added new imports and a type definition to make our code more structured.
- We've implemented a caching system to avoid loading the same SVGs multiple times.
- We're now using React hooks (useState and useEffect) to manage our component's state and side effects.
- We've improved our rendering logic to prevent errors.
All these changes should make our component faster and easier to maintain. It's like we've tuned up our code engine!
Now, for your homework: I want you to think about how we could measure the performance improvement. Any ideas? Here's a hint:
This script will help us measure the loading time before and after our changes. Remember, class: always verify your optimizations!
app/client/craco.common.config.js (1)
5-5: Very good, class! Let's examine this new import statement.I see you've added a new tool to our configuration toolkit. The BundleAnalyzerPlugin is an excellent choice for understanding our bundle composition. Well done on using the correct import syntax!
app/client/package.json (2)
243-243: Class, let's examine this new addition to our project's toolkit!I'm pleased to see that you've added the webpack-bundle-analyzer to our dependencies. This tool will be instrumental in helping us visualize the size of our webpack output files. It's like having a microscope for our code bundles!
Here's a quick lesson on why this is important:
- It helps us identify large dependencies that might be slowing down our application.
- We can use it to find duplicated modules across chunks.
- It assists in optimizing our code splitting strategy.
Remember, a smaller bundle size often leads to faster load times, which is crucial for a good user experience. Keep up the good work!
Line range hint
1-524: Class, let's recap today's lesson on package management!Today, we've learned about an important addition to our project: the webpack-bundle-analyzer. This tool is like a powerful magnifying glass for our code bundles. By adding it to our dependencies, we're taking a step towards better understanding and optimizing our application's performance.
Remember these key points:
- This change aligns perfectly with our goal of decoupling SVG imports from the main bundle.
- Using this analyzer can help us identify opportunities for code splitting and optimization.
- Smaller, more efficient bundles lead to faster load times and a better user experience.
For your homework, I want you to think about how we might use this tool in our development process. How can we integrate bundle analysis into our regular workflow?
Keep up the excellent work, and don't forget: in the world of web development, every kilobyte counts!
| 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); | ||
| }; |
There was a problem hiding this comment.
Time for a pop quiz on asynchronous operations!
Now, class, let's look at how we're loading our SVG icons. We're using some fancy React hooks called useState and useEffect. Can anyone tell me what these do?
Very good! useState helps us remember things, and useEffect helps us do things at the right time.
But wait a minute... I see something missing. Can anyone spot it? That's right, we're not handling errors! What if something goes wrong when we're trying to load our icons?
Let's improve this, shall we? Here's a little homework for you:
useEffect(() => {
const loadScript = async () => {
try {
const svgImportsMap = await loadSvgImportsMapOnce();
if (typeof icon === "string" && icon in svgImportsMap) {
const SvgIcon = await importSvg(svgImportsMap[icon][pixelGridSize]);
setSvgIcon(() => SvgIcon);
}
} catch (error) {
console.error("Error loading SVG icon:", error);
setSvgIcon(null);
}
};
loadScript();
return () => {
setSvgIcon(null);
};
}, [icon, pixelGridSize]);Can you see how we've added a try-catch block? This is like a safety net for our code. If anything goes wrong, we'll know about it and our program won't crash. Always remember to handle your errors, class!
app/client/craco.common.config.js
Outdated
| @@ -141,6 +142,7 @@ module.exports = { | |||
| "./src/components/designSystems/blueprintjs/icon/index.js", | |||
| ), | |||
| ), | |||
| new BundleAnalyzerPlugin() | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Now, let's turn our attention to how we're using our new tool.
I appreciate your enthusiasm in adding the BundleAnalyzerPlugin to our webpack configuration. However, we need to be more specific about when and how we use it. Remember, in software development, context is key!
Consider this homework assignment:
- Configure the plugin to only run in development mode.
- Set it to generate static reports instead of starting a server.
Here's a hint to get you started:
new BundleAnalyzerPlugin({
analyzerMode: process.env.NODE_ENV === 'production' ? 'disabled' : 'static',
openAnalyzer: false,
reportFilename: 'bundle-report.html'
})This way, we avoid any surprises in our production environment while still benefiting from the analysis during development. Don't forget to submit your work for review!
There was a problem hiding this comment.
This should affect the speed of the dev build, right? Can we put this under the env flag? To run only when necessary.
There was a problem hiding this comment.
@KelvinOm, that's an excellent point! Including the BundleAnalyzerPlugin can indeed affect the speed of the development build. To ensure it runs only when necessary, we can conditionally include it based on an environment flag.
Here's how you can modify your webpack configuration:
// At the top of your configuration file
const shouldAnalyzeBundle = process.env.ANALYZE_BUNDLE === 'true';
// Inside your webpack configuration
plugins: [
// ... other plugins
// Conditionally add the BundleAnalyzerPlugin
...(shouldAnalyzeBundle
? [
new BundleAnalyzerPlugin({
analyzerMode: 'static',
openAnalyzer: false,
reportFilename: 'bundle-report.html',
}),
]
: []),
],With this setup, the BundleAnalyzerPlugin will only be included when you set the ANALYZE_BUNDLE environment variable to true. This way, you maintain optimal build performance during regular development and enable the bundle analysis only when needed.
To generate the bundle analysis report, you can run:
ANALYZE_BUNDLE=true npm run buildUtilizing environment variables like this helps keep your development workflow efficient while providing flexibility for in-depth analysis when necessary.
Feel free to ask if you have any questions or need further clarification!
✏️ Learnings added
Learnt from: KelvinOm
PR: appsmithorg/appsmith#36662
File: app/client/craco.common.config.js:145-145
Timestamp: 2024-10-03T09:47:17.379Z
Learning: When adding the BundleAnalyzerPlugin, we should conditionally include it using an environment variable to prevent slowing down the development build.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
vsvamsi1
added
ok-to-test
|
/build-deploy-preview env=release |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/11159678946. |
app/client/craco.common.config.js
Outdated
| @@ -141,6 +142,7 @@ module.exports = { | |||
| "./src/components/designSystems/blueprintjs/icon/index.js", | |||
| ), | |||
| ), | |||
| new BundleAnalyzerPlugin() | |||
There was a problem hiding this comment.
This should affect the speed of the dev build, right? Can we put this under the env flag? To run only when necessary.
app/client/craco.dev.config.js
Outdated
| @@ -11,9 +11,6 @@ module.exports = merge(common, { | |||
| }, | |||
| }, | |||
| }, | |||
| optimization: { | |||
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/11159767128. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
app/client/src/components/designSystems/blueprintjs/icon/Icon.tsx (2)
32-46: Let's learn about caching and lazy loading!Good morning, class! Today, we're going to talk about two very important concepts in programming: caching and lazy loading.
First, let's look at line 33. We're creating a variable called
cachedSvgImportsMap. This is like a big box where we're going to store our SVG imports once we've loaded them. Why do we do this? Well, it's like packing your school bag the night before - you only need to do it once, and then you're ready to go in the morning!Now, let's move on to our
loadSvgImportsMapOncefunction. This function is practicing what we call "lazy loading". It's like waiting to pack your gym clothes until you know for sure you have gym class. We only load the SVG imports when we actually need them.But class, I think we're missing something important here. Can anyone tell me what it is? That's right - error handling! What if something goes wrong while we're loading our SVGs? We should be prepared for that.
Here's a little homework for you. Try adding a try-catch block to handle any errors that might occur during loading. It might look something like this:
const loadSvgImportsMapOnce = async () => { if (!cachedSvgImportsMap) { try { const { default: svgImportsMap } = await import( "components/designSystems/blueprintjs/icon/svgImportsMap" ); cachedSvgImportsMap = svgImportsMap; } catch (error) { console.error("Error loading SVG imports map:", error); // You might want to set cachedSvgImportsMap to an empty object or handle the error in some way } } return cachedSvgImportsMap; };Remember, class: always be prepared for things to go wrong. It's like carrying an umbrella - you might not need it, but you'll be glad you have it if it starts raining!
Line range hint
68-119: Time for a lesson on React hooks and conditional rendering!Good morning, class! Today, we're going to talk about some advanced React concepts. Let's start with the
useEffecthook on line 68.Can anyone tell me what
useEffectdoes? That's right, it lets us perform side effects in our components. In this case, we're using it to load our SVG icon. It's like a chef preparing ingredients before cooking - we're getting everything ready before we render our component.Now, let's look at our
loadScriptfunction insideuseEffect. It's doing some important work:
- It's loading our SVG imports map.
- It's checking if we have the right icon.
- If we do, it's loading that icon and setting it in our state.
But class, I think we're missing something important here. Can anyone tell me what it is? That's right - error handling! What if something goes wrong while we're loading our icon? We should be prepared for that.
Here's a little homework for you. Try adding a try-catch block to handle any errors that might occur during loading. It might look something like this:
const loadScript = async () => { try { const svgImportsMap = await loadSvgImportsMapOnce(); if (typeof icon === "string" && icon in svgImportsMap) { const SvgIcon = await importSvg(svgImportsMap[icon][pixelGridSize]); setSvgIcon(() => SvgIcon); } } catch (error) { console.error("Error loading SVG icon:", error); // You might want to set a default icon or handle the error in some way } };Now, let's move on to our rendering logic starting at line 111. Can anyone tell me what's special about how we're rendering our
SvgIcon? That's right, we're using conditional rendering!We're saying, "Only render the
SvgIconif it exists." It's like checking if you have your homework before trying to hand it in - very smart!Remember, class: always handle your errors and check if things exist before you use them. It's like looking both ways before you cross the street - it might seem unnecessary, but it can save you from a lot of trouble!
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- app/client/src/components/designSystems/blueprintjs/icon/Icon.tsx (4 hunks)
🔇 Additional comments (3)
app/client/src/components/designSystems/blueprintjs/icon/Icon.tsx (3)
3-8: Class, let's review our import statement!Good morning, students! Today, we're going to learn about importing React hooks. Can anyone tell me what new hooks we've added to our import statement?
That's right! We've added
useEffectanduseState. These are very important tools in our React toolbox.useEffecthelps us perform side effects in our components, whileuseStateallows us to add state to our functional components.We've also imported some types:
ComponentTypeandSVGProps. These will help us write more type-safe code. Remember, class: type safety is like wearing a helmet when you're riding a bike - it might seem unnecessary, but it can save you from a lot of headaches later!
19-30: Time for a pop quiz on TypeScript types!Alright, class, who can tell me what we've added here? That's correct, we've imported
IconNameand created a new type calledIconMapType.Now, let's break down this
IconMapType. It's like a big dictionary where we can look up our SVG icons. The keys are the icon names, and the values? Well, they're another dictionary! This inner dictionary tells us where to find the right SVG file based on the icon size.Notice how we're using
Recordhere? That's a fancy TypeScript way of saying "this is an object with specific types for its keys and values". It's like giving our object a very strict set of rules to follow.And see that
Promise<typeof import("*.svg")>? That's telling us that loading these SVGs is going to be an asynchronous operation. It's like ordering a pizza - you place the order now, but the pizza (or in our case, the SVG) will arrive a bit later!Great job, everyone! This type definition will help us keep our icon loading organized and type-safe.
60-62: Let's dive into React state!Good morning, class! Today, we're going to talk about React's
useStatehook. Can anyone tell me what we're doing in these lines?That's right! We're creating a new state variable called
SvgIcon. But this isn't just any ordinary state variable. Look at those angle brackets - we're using TypeScript to tell React exactly what type of dataSvgIconcan be.In this case,
SvgIconcan either be aComponentType<SVGProps<SVGSVGElement>>ornull. That's a mouthful, isn't it? Let's break it down:
ComponentTypemeans it's a React component.SVGProps<SVGSVGElement>means it's specifically an SVG component.nullmeans that when we don't have an icon loaded,SvgIconwill benull.This is great because it helps us avoid errors later on. It's like labeling your pencil case - you always know what's supposed to be inside!
|
Deploy-Preview-URL: https://ce-36662.dp.appsmith.com |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
app/client/cypress/e2e/Regression/ClientSide/Performance/LinkRelPreload_Spec.js (1)
135-137: A gold star for improving the comparison logic!Your update to the comparison logic is like upgrading from a abacus to a calculator - much more efficient! Using
everyto check if allpreloadLinksare included inallRequestsDuringPageLoadis a smart move. It's both more readable and more performant.However, let's make it even clearer for future readers:
const allPreloadLinksAreLoaded = preloadLinks.every(item => allRequestsDuringPageLoad.includes(item)); expect(allPreloadLinksAreLoaded, "All preload links should be loaded").to.be.true;This way, anyone reading the test will immediately understand what we're asserting. Remember, in testing, clarity is key!
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- app/client/cypress/e2e/Regression/ClientSide/Performance/LinkRelPreload_Spec.js (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
app/client/cypress/e2e/Regression/ClientSide/Performance/LinkRelPreload_Spec.js (1)
Pattern
app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
🔇 Additional comments (2)
app/client/cypress/e2e/Regression/ClientSide/Performance/LinkRelPreload_Spec.js (2)
115-123: Well done on improving variable naming, class!The renaming of
requestsToComparetoallRequestsDuringPageLoadis a step in the right direction. It makes the code more self-explanatory, which is crucial for maintaining clear and understandable tests. Keep up the good work!
Line range hint
123-134: Excellent job on continuing to improve variable names!Your decision to rename
linksToComparetopreloadLinksis commendable. It's like putting a clear label on a jar - everyone knows exactly what's inside! This change enhances the readability of our test code. Remember, clear naming is half the battle in writing maintainable tests.
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 Numberor
Fixes
Issue URLWarning
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
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11178699547
Commit: c6ee0c3
Cypress dashboard.
Tags:
@tag.AllSpec:
Fri, 04 Oct 2024 13:28:17 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Iconcomponent to load SVG icons asynchronously, improving efficiency and responsiveness.webpack-bundle-analyzer, to analyze bundle size and structure during the build process.Bug Fixes
Chores
devServerconfiguration to suppress warnings and errors in the client overlay.