Skip to content

Commit

Permalink
chore: decouple widget-config.json from main chunk (#36924)
Browse files Browse the repository at this point in the history
## Description
We decoupled the widget-config.json file (177KB when zipped) from the
main bundle and implemented lazy loading during DSL migrations. This
optimisation reduces the size of the main chunk by 10%, which should
lead to better performance, specifically improving FCP and LCP, as the
main chunk's fetch and evaluation times are reduced

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/11384816747>
> Commit: ae14b3f
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11384816747&attempt=2"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.All`
> Spec:
> <hr>Thu, 17 Oct 2024 15:04:34 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 migration logic for DSL widgets with asynchronous operations.
	- Improved performance with lazy loading of widget configurations.
	- Added optional properties to page response data structures.

- **Bug Fixes**
	- Resolved issues with dynamic binding path migrations.

- **Tests**
- Updated test cases to handle asynchronous operations for better
reliability and accuracy.

- **Chores**
- Improved type safety and error handling in widget factory utilities
and mock functions.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
vsvamsi1 authored Oct 17, 2024
1 parent ebec99c commit b122c81
Show file tree
Hide file tree
Showing 21 changed files with 401 additions and 244 deletions.
12 changes: 6 additions & 6 deletions app/client/packages/dsl/src/migrate/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ const migrateUnversionedDSL = (currentDSL: DSLWidget) => {
// A rudimentary transform function which updates the DSL based on its version.
// A more modular approach needs to be designed.
// This needs the widget config to be already built to migrate correctly
const migrateVersionedDSL = (currentDSL: DSLWidget, newPage = false) => {
const migrateVersionedDSL = async (currentDSL: DSLWidget, newPage = false) => {
if (currentDSL.version === 1) {
if (currentDSL.children && currentDSL.children.length > 0)
currentDSL.children = currentDSL.children.map(updateContainers);
Expand Down Expand Up @@ -205,7 +205,7 @@ const migrateVersionedDSL = (currentDSL: DSLWidget, newPage = false) => {
}

if (currentDSL.version === 12) {
currentDSL = migrateIncorrectDynamicBindingPathLists(currentDSL);
currentDSL = await migrateIncorrectDynamicBindingPathLists(currentDSL);
currentDSL.version = 13;
}

Expand Down Expand Up @@ -619,15 +619,15 @@ const migrateVersionedDSL = (currentDSL: DSLWidget, newPage = false) => {
return currentDSL;
};

export const migrateDSL = (
export const migrateDSL = async (
currentDSL: DSLWidget,
newPage = false,
): DSLWidget => {
): Promise<DSLWidget> => {
if (currentDSL.version === undefined) {
const initialDSL = migrateUnversionedDSL(currentDSL);

return migrateVersionedDSL(initialDSL, newPage) as DSLWidget;
return (await migrateVersionedDSL(initialDSL, newPage)) as DSLWidget;
} else {
return migrateVersionedDSL(currentDSL, newPage) as DSLWidget;
return (await migrateVersionedDSL(currentDSL, newPage)) as DSLWidget;
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import isString from "lodash/isString";
import memoize from "micro-memoize";
import { isObject, isUndefined } from "lodash";
import { generateReactKey, isDynamicValue } from "../utils";
import widgetConfigs from "../helpers/widget-configs.json";

export const WidgetHeightLimits = {
MAX_HEIGHT_IN_ROWS: 9000,
Expand Down Expand Up @@ -511,8 +510,33 @@ export function addSearchConfigToPanelConfig(config: readonly any[]) {
return configItem;
});
}
// Cache for lazy-loaded widget configurations
let cachedWidgetConfigs: any | null = null;

/*
Lazily load this file since it is very large and used in migrations for certain DSL versions. By lazily loading
this large file it can be reduce the main chunk only be loaded for certain limited conditions.
*/
const loadWidgetConfig = async () => {
if (!cachedWidgetConfigs) {
try {
const { default: widgetConfigs } = await import(
"../helpers/widget-configs.json"
);

cachedWidgetConfigs = widgetConfigs; // Cache the module for future use
} catch (e) {
log.error("Error loading WidgetConfig", e);
}
}

const getWidgetPropertyPaneContentConfig = (type: string): readonly any[] => {
return cachedWidgetConfigs;
};

const getWidgetPropertyPaneContentConfig = async (
type: string,
): Promise<readonly any[]> => {
const widgetConfigs = await loadWidgetConfig();
const propertyPaneContentConfig = (widgetConfigs as any)[type]
.propertyPaneContentConfig;

Expand Down Expand Up @@ -540,7 +564,11 @@ const getWidgetPropertyPaneContentConfig = (type: string): readonly any[] => {
}
};

const getWidgetPropertyPaneStyleConfig = (type: string): readonly any[] => {
const getWidgetPropertyPaneStyleConfig = async (
type: string,
): Promise<readonly any[]> => {
const widgetConfigs = await loadWidgetConfig();

const propertyPaneStyleConfig = (widgetConfigs as any)[type]
.propertyPaneStyleConfig;

Expand All @@ -567,14 +595,20 @@ const getWidgetPropertyPaneStyleConfig = (type: string): readonly any[] => {
}
};

const getWidgetPropertyPaneCombinedConfig = (type: string): readonly any[] => {
const contentConfig = getWidgetPropertyPaneContentConfig(type);
const styleConfig = getWidgetPropertyPaneStyleConfig(type);
const getWidgetPropertyPaneCombinedConfig = async (
type: string,
): Promise<readonly any[]> => {
const contentConfig = await getWidgetPropertyPaneContentConfig(type);
const styleConfig = await getWidgetPropertyPaneStyleConfig(type);

return [...contentConfig, ...styleConfig];
};

const getWidgetPropertyPaneConfig = (type: string): readonly any[] => {
const getWidgetPropertyPaneConfig = async (
type: string,
): Promise<readonly any[]> => {
const widgetConfigs = await loadWidgetConfig();

const propertyPaneConfig = (widgetConfigs as any)[type].propertyPaneConfig;

const features = (widgetConfigs as any)[type].features;
Expand All @@ -590,7 +624,7 @@ const getWidgetPropertyPaneConfig = (type: string): readonly any[] => {

return enhancedPropertyPaneConfig;
} else {
const config = getWidgetPropertyPaneCombinedConfig(type);
const config = await getWidgetPropertyPaneCombinedConfig(type);

if (config === undefined) {
log.error("Widget property pane config not defined", type);
Expand Down Expand Up @@ -957,14 +991,14 @@ const getAllPathsFromPropertyConfig = memoize(
{ maxSize: 1000 },
);

export const migrateIncorrectDynamicBindingPathLists = (
export const migrateIncorrectDynamicBindingPathLists = async (
currentDSL: Readonly<DSLWidget>,
): DSLWidget => {
): Promise<DSLWidget> => {
const migratedDsl = {
...currentDSL,
};
const dynamicBindingPathList: any[] = [];
const propertyPaneConfig = getWidgetPropertyPaneConfig(currentDSL.type);
const propertyPaneConfig = await getWidgetPropertyPaneConfig(currentDSL.type);
const { bindingPaths } = getAllPathsFromPropertyConfig(
currentDSL,
propertyPaneConfig,
Expand All @@ -984,8 +1018,10 @@ export const migrateIncorrectDynamicBindingPathLists = (
migratedDsl.dynamicBindingPathList = dynamicBindingPathList;

if (currentDSL.children) {
migratedDsl.children = currentDSL.children.map(
migrateIncorrectDynamicBindingPathLists,
migratedDsl.children = await Promise.all(
currentDSL.children.map(async (value) =>
migrateIncorrectDynamicBindingPathLists(value),
),
);
}

Expand Down
127 changes: 62 additions & 65 deletions app/client/packages/dsl/src/migrate/tests/DSLMigration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -938,85 +938,82 @@ describe("Test all the migrations are running", () => {
afterAll(() => {
jest.clearAllMocks();
});
migrations.forEach((migration: Migration) => {
/**
* Generates mock fucntion for each migration function.
* Mocks the implementation
*/
const version = migration.version ?? 0;
test("assert migration functions being called when migrate dsl has been called ", async () => {
migrations.forEach((migration: Migration) => {
/**
* Generates mock fucntion for each migration function.
* Mocks the implementation
*/
const version = migration.version ?? 0;

mockFnObj[version] = [];
mockFnObj[version] = [];

migration.functionLookup.forEach((lookup) => {
const { functionName, moduleObj } = lookup;
migration.functionLookup.forEach((lookup) => {
const { functionName, moduleObj } = lookup;

if (moduleObj) {
mockFnObj[version].push({
spyOnFunc: jest
.spyOn(moduleObj, functionName)
.mockImplementation((dsl: any) => {
/**
* We need to delete the children property on the first migration(calculateDynamicHeight),
* to avoid the recursion in the second migration(updateContainers)
*/
dsl && delete dsl.children;
if (moduleObj) {
mockFnObj[version].push({
spyOnFunc: jest
.spyOn(moduleObj, functionName)
.mockImplementation((dsl: any) => {
/**
* We need to delete the children property on the first migration(calculateDynamicHeight),
* to avoid the recursion in the second migration(updateContainers)
*/
dsl && delete dsl.children;

return {
version: dsl?.version,
validationFuncName: functionName,
};
}),
});
}
return {
version: dsl?.version,
validationFuncName: functionName,
};
}),
});
}
});
});
});

// Runs all the migrations
DSLMigrations.migrateDSL(originalDSLForDSLMigrations as unknown as DSLWidget);
// Runs all the migrations
await DSLMigrations.migrateDSL(
originalDSLForDSLMigrations as unknown as DSLWidget,
);

migrations.forEach((item: any, testIdx: number) => {
const { functionLookup, version } = item;
const dslVersion = version ?? 0;
migrations.forEach((item: any) => {
const { functionLookup, version } = item;
const dslVersion = version ?? 0;

functionLookup.forEach(
(lookup: { moduleObj: any; functionName: string }, index: number) => {
const { functionName, moduleObj } = lookup;
functionLookup.forEach(
(lookup: { moduleObj: any; functionName: string }, index: number) => {
const { functionName, moduleObj } = lookup;

if (moduleObj) {
const mockObj = mockFnObj[dslVersion][index].spyOnFunc;
const calls = mockObj.mock?.calls;
const results = mockObj.mock?.results;
const resultsLastIdx = mockObj.mock.results.length - 1;

describe(`Test ${testIdx}:`, () => {
test(`Has ${functionName} function executed?`, () => {
// Check if the migration function is called
expect(results[resultsLastIdx].value.validationFuncName).toEqual(
functionName,
);
});
if (moduleObj) {
const mockObj = mockFnObj[dslVersion][index].spyOnFunc;
const calls = mockObj.mock?.calls;
const results = mockObj.mock?.results;
const resultsLastIdx = mockObj.mock.results.length - 1;

// Check if the migration function is called
expect(results[resultsLastIdx].value.validationFuncName).toEqual(
functionName,
);
// Check if the migration function is called with the current DSL version
calls.forEach((args: any) => {
test(`Does ${functionName} executes with DSL version: ${version}?`, () => {
if (args[0]?.version === version) {
expect(args[0]?.version).toEqual(version);
}
});
test(`For ${functionName}, is the ${args[0]?.version} registerd in tests?`, () => {
expect(
Object.keys(mockFnObj).includes(
args[0]?.version.toString() ?? "0",
),
).toBe(true);
});
// Does functionName executes with the correct DSL version number
if (args[0]?.version === version) {
expect(args[0]?.version).toEqual(version);
}

// For a functionName is the correct DSL version registed in test cases
expect(
Object.keys(mockFnObj).includes(
args[0]?.version.toString() ?? "0",
),
).toBe(true);
});
});
}
},
);
}
},
);
});
});

test("Check the migration count matches the lates page version", () => {
expect(migrations.length).toEqual(DSLMigrations.LATEST_DSL_VERSION);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type { DSLWidget } from "../types";
const ASSETS_CDN_URL = "https://assets.appsmith.com";

describe("correctly migrate dsl", () => {
it("migrateDSL for private widget", () => {
it("migrateDSL for private widget", async () => {
const currentVersion = 49; // before adding privateWidgets to all List widgets
const nextVersion = LATEST_DSL_VERSION; // It runs Two Migrations, Always Update as migration increases

Expand Down Expand Up @@ -1229,12 +1229,12 @@ describe("correctly migrate dsl", () => {
isLoading: false,
};

const actualNextDsl = migrateDSL(currentDSL);
const actualNextDsl = await migrateDSL(currentDSL);

expect(actualNextDsl).toEqual(expectedNextDSL);
});

it("migrateDSL for theming v1", () => {
it("migrateDSL for theming v1", async () => {
const currentVersion = 53;
const nextVersion = LATEST_DSL_VERSION;
const currentDSL: DSLWidget = {
Expand Down Expand Up @@ -2452,7 +2452,7 @@ describe("correctly migrate dsl", () => {
isLoading: false,
};

const actualNextDsl = migrateDSL(currentDSL);
const actualNextDsl = await migrateDSL(currentDSL);

expect(actualNextDsl).toEqual(expectedNextDSL);
});
Expand Down Expand Up @@ -3096,7 +3096,7 @@ describe("correctly migrate dsl", () => {
expect(actualNextDSL).toEqual(expectedNextDSL);
});

it("correctly migrates currentIndex/currentRow properties for validations in table view", () => {
it("correctly migrates currentIndex/currentRow properties for validations in table view", async () => {
const currentVersion = 78;
const currentDSL: DSLWidget = {
bottomRow: 740,
Expand Down Expand Up @@ -3171,7 +3171,7 @@ describe("correctly migrate dsl", () => {
},
],
};
const nextDSL = migrateDSL(currentDSL);
const nextDSL = await migrateDSL(currentDSL);

// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const validations = (nextDSL.children || [])[0].primaryColumns.column1
Expand Down
4 changes: 2 additions & 2 deletions app/client/packages/rts/src/controllers/Dsl/DslController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ export default class DSLController extends BaseController {
super();
}

migrateDSL(req: Request, res: Response) {
async migrateDSL(req: Request, res: Response) {
try {
const latestDSL = migrateDSLToLatest(req.body);
const latestDSL = await migrateDSLToLatest(req.body);

super.sendResponse(res, latestDSL);
} catch (err) {
Expand Down
8 changes: 7 additions & 1 deletion app/client/packages/rts/src/routes/dsl_routes.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import DSLController from "@controllers/Dsl/DslController";
import { Validator } from "@middlewares/Validator";
import express from "express";
import type { Response, Request } from "express";

const router = express.Router();
const dslController = new DSLController();
Expand All @@ -12,6 +13,11 @@ router.get(
dslController.getLatestDSLVersion,
);

router.post("/migrate", validator.validateRequest, dslController.migrateDSL);
router.post(
"/migrate",
validator.validateRequest,
async (req: Request, res: Response) =>
await dslController.migrateDSL(req, res),
);

export default router;
Loading

0 comments on commit b122c81

Please sign in to comment.