From e9ec8d227f439f04138980db7f8d6152ec183501 Mon Sep 17 00:00:00 2001 From: arunvjn <32433245+arunvjn@users.noreply.github.com> Date: Fri, 1 Sep 2023 13:49:21 +0530 Subject: [PATCH] feat: Added support for ESM builds of custom JS libraries (#26671) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Appsmith so far could only support UMD builds of any JS library. This meant that users would have to figure a compatible UMD build that works with Appsmith and test if works within Appsmith. Most libraries on jsDelivr have ESM builds shipped out of the box and support these libraries should take away the UMD pain from the users. #### PR fixes following issue(s) Fixes #26424 > #### Media > A video or a GIF is preferred. when using Loom, don’t embed because it looks like it’s a GIF. instead, just link to the video > > #### Type of change - New feature (non-breaking change which adds functionality) > > ## Testing > #### How Has This Been Tested? - [x] Manual - [x] Cypress > > #### Test Plan - [x] Installation/uninstallation - [x] lint error check post uninstall - [x] Installation/uninstallation on git connected app - [x] edit & view mode check - [x] incorrect URL format > > #### Issues raised during DP testing None **caveat** - Libraries that require to be run on worker will fail eg: https://cdn.jsdelivr.net/npm/@javfres/speech-generator@1.0.5/+esm > > > ## Checklist: #### Dev activity - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [x] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [x] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [x] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed --------- Co-authored-by: Aishwarya UR --- .../ClientSide/JSLibrary/Library_spec.ts | 27 +++- .../cypress/support/Pages/LibraryInstaller.ts | 17 ++ .../workers/Evaluation/evalWorkerActions.ts | 4 +- app/client/src/sagas/JSLibrarySaga.ts | 17 ++ .../handlers/__tests__/jsLibrary.test.ts | 30 ++-- .../src/workers/Evaluation/handlers/index.ts | 4 +- .../workers/Evaluation/handlers/jsLibrary.ts | 152 ++++++++++++++---- 7 files changed, 199 insertions(+), 52 deletions(-) diff --git a/app/client/cypress/e2e/Regression/ClientSide/JSLibrary/Library_spec.ts b/app/client/cypress/e2e/Regression/ClientSide/JSLibrary/Library_spec.ts index 4dcfb009cf..4c77d0fabf 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/JSLibrary/Library_spec.ts +++ b/app/client/cypress/e2e/Regression/ClientSide/JSLibrary/Library_spec.ts @@ -45,7 +45,28 @@ describe("excludeForAirgap", "Tests JS Libraries", () => { _.debuggerHelper.ClickResponseTab(); _.agHelper.AssertContains("data:application/pdf;filename=generated.pdf"); }); - it("4. Shows list of recommended libraries", () => { + + it("4. ESM build should pass installation, uninstallation and reinstallation", () => { + _.entityExplorer.ExpandCollapseEntity("Libraries"); + _.installer.OpenInstaller(); + _.installer.installLibraryViaURL( + "https://cdn.jsdelivr.net/npm/fast-xml-parser@4.2.7/+esm", + "fast_xml_parser", + ); + _.agHelper.Sleep(2000); + // Uninstallation should succeed + _.installer.uninstallLibrary("fast_xml_parser"); + _.installer.assertUnInstall("fast_xml_parser"); + + // Reinstallation should succeed with the same accessor + _.installer.OpenInstaller(); + _.installer.installLibraryViaURL( + "https://cdn.jsdelivr.net/npm/fast-xml-parser@4.2.7/+esm", + "fast_xml_parser", + ); + }); + + it("5. Shows list of recommended libraries", () => { const recommendedLibraryNames = ["jsonwebtoken", "jspdf", "bcryptjs"]; _.entityExplorer.ExpandCollapseEntity("Libraries"); _.installer.OpenInstaller(); @@ -54,7 +75,7 @@ describe("excludeForAirgap", "Tests JS Libraries", () => { } }); - it("5. Checks installation in exported/forked app", () => { + it("6. Checks installation in exported/forked app", () => { _.homePage.NavigateToHome(); _.homePage.ImportApp("library_export.json"); _.agHelper.AssertContains("true"); @@ -73,7 +94,7 @@ describe("excludeForAirgap", "Tests JS Libraries", () => { _.agHelper.AssertContains("true"); }); - it("6. Tests library access and installation in public apps", () => { + it("7. Tests library access and installation in public apps", () => { let appURL = ""; cy.get(HomePage.shareApp).click(); //@ts-expect-error no type access diff --git a/app/client/cypress/support/Pages/LibraryInstaller.ts b/app/client/cypress/support/Pages/LibraryInstaller.ts index fffe6b042e..e6e50de158 100644 --- a/app/client/cypress/support/Pages/LibraryInstaller.ts +++ b/app/client/cypress/support/Pages/LibraryInstaller.ts @@ -15,6 +15,9 @@ export class LibraryInstaller { return `div.library-card.t--${libraryName}`; } + private libraryURLLocator = "[data-testid='library-url']"; + private installBtnLocator = "[data-testid='install-library-btn']"; + public OpenInstaller(force = false) { this._aggregateHelper.GetNClick(this._installer_trigger_locator, 0, force); } @@ -34,6 +37,20 @@ export class LibraryInstaller { if (checkIfSuccessful) this.assertInstall(libraryName, accessor); } + public installLibraryViaURL( + url: string, + accessor: string, + checkIfSuccessful = true, + ) { + this._aggregateHelper.TypeText(this.libraryURLLocator, url); + this._aggregateHelper.GetNClick(this.installBtnLocator); + if (checkIfSuccessful) { + this._aggregateHelper.AssertContains( + `Installation Successful. You can access the library via ${accessor}`, + ); + } + } + private assertInstall(libraryName: string, accessor: string) { this._aggregateHelper.AssertContains( `Installation Successful. You can access the library via ${accessor}`, diff --git a/app/client/src/ce/workers/Evaluation/evalWorkerActions.ts b/app/client/src/ce/workers/Evaluation/evalWorkerActions.ts index 2a6bcbd8dd..7671120611 100644 --- a/app/client/src/ce/workers/Evaluation/evalWorkerActions.ts +++ b/app/client/src/ce/workers/Evaluation/evalWorkerActions.ts @@ -9,15 +9,15 @@ export enum EVAL_WORKER_SYNC_ACTION { UPDATE_REPLAY_OBJECT = "UPDATE_REPLAY_OBJECT", SET_EVALUATION_VERSION = "SET_EVALUATION_VERSION", INIT_FORM_EVAL = "INIT_FORM_EVAL", - INSTALL_LIBRARY = "INSTALL_LIBRARY", UNINSTALL_LIBRARY = "UNINSTALL_LIBRARY", - LOAD_LIBRARIES = "LOAD_LIBRARIES", LINT_TREE = "LINT_TREE", } export enum EVAL_WORKER_ASYNC_ACTION { EVAL_TRIGGER = "EVAL_TRIGGER", EVAL_EXPRESSION = "EVAL_EXPRESSION", + LOAD_LIBRARIES = "LOAD_LIBRARIES", + INSTALL_LIBRARY = "INSTALL_LIBRARY", } export const EVAL_WORKER_ACTIONS = { diff --git a/app/client/src/sagas/JSLibrarySaga.ts b/app/client/src/sagas/JSLibrarySaga.ts index e0c46a2ea8..ac99b3d201 100644 --- a/app/client/src/sagas/JSLibrarySaga.ts +++ b/app/client/src/sagas/JSLibrarySaga.ts @@ -85,6 +85,23 @@ export function* installLibrarySaga(lib: Partial) { selectInstalledLibraries, ); + const alreadyInstalledLibrary = installedLibraries.find( + (library) => library.url === url, + ); + + if (alreadyInstalledLibrary) { + toast.show( + createMessage( + customJSLibraryMessages.INSTALLED_ALREADY, + alreadyInstalledLibrary.accessor, + ), + { + kind: "info", + }, + ); + return; + } + const takenAccessors = ([] as string[]).concat( ...installedLibraries.map((lib) => lib.accessor), ); diff --git a/app/client/src/workers/Evaluation/handlers/__tests__/jsLibrary.test.ts b/app/client/src/workers/Evaluation/handlers/__tests__/jsLibrary.test.ts index 4658ee550a..8ee15ccfb7 100644 --- a/app/client/src/workers/Evaluation/handlers/__tests__/jsLibrary.test.ts +++ b/app/client/src/workers/Evaluation/handlers/__tests__/jsLibrary.test.ts @@ -1,5 +1,8 @@ import { installLibrary, uninstallLibrary } from "../jsLibrary"; -import { EVAL_WORKER_SYNC_ACTION } from "@appsmith/workers/Evaluation/evalWorkerActions"; +import { + EVAL_WORKER_ASYNC_ACTION, + EVAL_WORKER_SYNC_ACTION, +} from "@appsmith/workers/Evaluation/evalWorkerActions"; import * as mod from "../../../common/JSLibrary/ternDefinitionGenerator"; jest.mock("../../../common/JSLibrary/ternDefinitionGenerator"); @@ -11,6 +14,9 @@ describe("Tests to assert install/uninstall flows", function () { self.lodash = {}; }); + //@ts-expect-error importScripts is not defined in the test environment + self.import = jest.fn(); + const mockTernDefsGenerator = jest.fn(() => ({})); jest.mock("../../../common/JSLibrary/ternDefinitionGenerator.ts", () => { @@ -20,14 +26,14 @@ describe("Tests to assert install/uninstall flows", function () { }); }); - it("should install a library", function () { - const res = installLibrary({ + it("should install a library", async function () { + const res = await installLibrary({ data: { url: "https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.21/lodash.min.js", takenAccessors: [], takenNamesMap: {}, }, - method: EVAL_WORKER_SYNC_ACTION.INSTALL_LIBRARY, + method: EVAL_WORKER_ASYNC_ACTION.INSTALL_LIBRARY, }); // expect(self.importScripts).toHaveBeenCalled(); @@ -43,14 +49,14 @@ describe("Tests to assert install/uninstall flows", function () { }); }); - it("Reinstalling a different version of the same installed library should fail", function () { - const res = installLibrary({ + it("Reinstalling a different version of the same installed library should fail", async function () { + const res = await installLibrary({ data: { url: "https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.16.0/lodash.min.js", takenAccessors: ["lodash"], takenNamesMap: {}, }, - method: EVAL_WORKER_SYNC_ACTION.INSTALL_LIBRARY, + method: EVAL_WORKER_ASYNC_ACTION.INSTALL_LIBRARY, }); expect(res).toEqual({ success: false, @@ -59,16 +65,16 @@ describe("Tests to assert install/uninstall flows", function () { }); }); - it("Detects name space collision where there is another entity(api, widget or query) with the same name", function () { + it("Detects name space collision where there is another entity(api, widget or query) with the same name", async function () { //@ts-expect-error ignore delete self.lodash; - const res = installLibrary({ + const res = await installLibrary({ data: { url: "https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.16.0/lodash.min.js", takenAccessors: [], takenNamesMap: { lodash: true }, }, - method: EVAL_WORKER_SYNC_ACTION.INSTALL_LIBRARY, + method: EVAL_WORKER_ASYNC_ACTION.INSTALL_LIBRARY, }); expect(res).toEqual({ success: false, @@ -77,10 +83,10 @@ describe("Tests to assert install/uninstall flows", function () { }); }); - it("Removes or set the accessors to undefined on the global object on uninstallation", function () { + it("Removes or set the accessors to undefined on the global object on uninstallation", async function () { //@ts-expect-error ignore self.lodash = {}; - const res = uninstallLibrary({ + const res = await uninstallLibrary({ data: ["lodash"], method: EVAL_WORKER_SYNC_ACTION.UNINSTALL_LIBRARY, }); diff --git a/app/client/src/workers/Evaluation/handlers/index.ts b/app/client/src/workers/Evaluation/handlers/index.ts index 7315c1a15f..cf0f6f6118 100644 --- a/app/client/src/workers/Evaluation/handlers/index.ts +++ b/app/client/src/workers/Evaluation/handlers/index.ts @@ -27,9 +27,7 @@ const syncHandlerMap: Record< [EVAL_WORKER_ACTIONS.REDO]: redo, [EVAL_WORKER_ACTIONS.UPDATE_REPLAY_OBJECT]: updateReplayObject, [EVAL_WORKER_ACTIONS.VALIDATE_PROPERTY]: validateProperty, - [EVAL_WORKER_ACTIONS.INSTALL_LIBRARY]: installLibrary, [EVAL_WORKER_ACTIONS.UNINSTALL_LIBRARY]: uninstallLibrary, - [EVAL_WORKER_ACTIONS.LOAD_LIBRARIES]: loadLibraries, [EVAL_WORKER_ACTIONS.LINT_TREE]: noop, [EVAL_WORKER_ACTIONS.SETUP]: setupEvaluationEnvironment, [EVAL_WORKER_ACTIONS.CLEAR_CACHE]: clearCache, @@ -43,6 +41,8 @@ const asyncHandlerMap: Record< > = { [EVAL_WORKER_ACTIONS.EVAL_TRIGGER]: evalTrigger, [EVAL_WORKER_ACTIONS.EVAL_EXPRESSION]: evalExpression, + [EVAL_WORKER_ACTIONS.LOAD_LIBRARIES]: loadLibraries, + [EVAL_WORKER_ACTIONS.INSTALL_LIBRARY]: installLibrary, }; export { syncHandlerMap, asyncHandlerMap }; diff --git a/app/client/src/workers/Evaluation/handlers/jsLibrary.ts b/app/client/src/workers/Evaluation/handlers/jsLibrary.ts index 0295bb396b..600d2ba89e 100644 --- a/app/client/src/workers/Evaluation/handlers/jsLibrary.ts +++ b/app/client/src/workers/Evaluation/handlers/jsLibrary.ts @@ -11,7 +11,7 @@ import { } from "../../common/JSLibrary"; import { resetJSLibraries } from "../../common/JSLibrary/resetJSLibraries"; import { makeTernDefs } from "../../common/JSLibrary/ternDefinitionGenerator"; -import type { EvalWorkerSyncRequest } from "../types"; +import type { EvalWorkerASyncRequest, EvalWorkerSyncRequest } from "../types"; import { dataTreeEvaluator } from "./evalTree"; enum LibraryInstallError { @@ -81,7 +81,7 @@ function addTempStoredDataTreeToContext( } } -export function installLibrary(request: EvalWorkerSyncRequest) { +export async function installLibrary(request: EvalWorkerASyncRequest) { const { data } = request; const { takenAccessors, takenNamesMap, url } = data; const defs: Def = {}; @@ -98,44 +98,58 @@ export function installLibrary(request: EvalWorkerSyncRequest) { const unsetKeys = currentEnvKeys.filter((key) => self[key] === undefined); const existingLibraries: Record = {}; - for (const acc of takenAccessors) { existingLibraries[acc] = self[acc]; } - + let module = null; try { self.importScripts(url); } catch (e) { - throw new ImportError(url); + try { + module = await import(/* webpackIgnore: true */ url); + } catch (e) { + throw new ImportError(url); + } } // Find keys add that were installed to the global scope. - const accessor = difference( + const accessors = difference( Object.keys(self), currentEnvKeys, ) as Array; + // If no keys were added to the global scope, check if the module is a ESM module. + if (accessors.length === 0) { + if (module && typeof module === "object") { + const uniqAccessor = generateUniqueAccessor( + url, + takenAccessors, + takenNamesMap, + ); + // @ts-expect-error no types + self[uniqAccessor] = flattenModule(module); + accessors.push(uniqAccessor); + } + } + addTempStoredDataTreeToContext(tempDataTreeStore); - - checkForNameCollision(accessor, takenNamesMap); - - checkIfUninstalledEarlier(accessor, unsetKeys); - - checkForOverrides(url, accessor, takenAccessors, existingLibraries); - - if (accessor.length === 0) return { status: false, defs, accessor }; + checkForNameCollision(accessors, takenNamesMap); + checkIfUninstalledEarlier(accessors, unsetKeys); + checkForOverrides(url, accessors, takenAccessors, existingLibraries); + if (accessors.length === 0) + return { status: false, defs, accessor: accessors }; //Reserves accessor names. - const name = accessor[accessor.length - 1]; + const name = accessors[accessors.length - 1]; defs["!name"] = `LIB/${name}`; try { - for (const key of accessor) { + for (const key of accessors) { //@ts-expect-error no types defs[key] = makeTernDefs(self[key]); } } catch (e) { - for (const acc of accessor) { + for (const acc of accessors) { //@ts-expect-error no types self[acc] = undefined; } @@ -145,13 +159,13 @@ export function installLibrary(request: EvalWorkerSyncRequest) { } //Reserve accessor names. - for (const acc of accessor) { + for (const acc of accessors) { //we have to update invalidEntityIdentifiers as well libraryReservedIdentifiers[acc] = true; invalidEntityIdentifiers[acc] = true; } - return { success: true, defs, accessor }; + return { success: true, defs, accessor: accessors }; } catch (error) { return { success: false, defs, error }; } @@ -178,28 +192,46 @@ export function uninstallLibrary(request: EvalWorkerSyncRequest) { } } -export function loadLibraries(request: EvalWorkerSyncRequest) { +export async function loadLibraries(request: EvalWorkerASyncRequest) { resetJSLibraries(); //Add types - const { data } = request; - const urls = data.map((lib: any) => lib.url); - const keysBefore = Object.keys(self); + const { data: libs } = request; let message = ""; try { - self.importScripts(...urls); + for (const lib of libs) { + const url = lib.url; + const accessor = lib.accessor; + const keysBefore = Object.keys(self); + let module = null; + try { + self.importScripts(url); + } catch (e) { + message = (e as Error).message; + try { + module = await import(/* webpackIgnore: true */ url); + } catch (e) { + message = (e as Error).message; + } + } + const keysAfter = Object.keys(self); + const newKeys = difference(keysAfter, keysBefore); + if (newKeys.length === 0 && module && typeof module === "object") { + self[accessor[0]] = flattenModule(module); + newKeys.push(accessor[0]); + } + for (const key of newKeys) { + //we have to update invalidEntityIdentifiers as well + libraryReservedIdentifiers[key] = true; + invalidEntityIdentifiers[key] = true; + } + } + JSLibraries.push(...libs); + return { success: true, message }; } catch (e) { message = (e as Error).message; + return { success: false, message }; } - const keysAfter = Object.keys(self); - const newKeys = difference(keysAfter, keysBefore); - for (const key of newKeys) { - //we have to update invalidEntityIdentifiers as well - libraryReservedIdentifiers[key] = true; - invalidEntityIdentifiers[key] = true; - } - JSLibraries.push(...data); - return { success: !message, message }; } function checkForNameCollision( @@ -243,3 +275,57 @@ function checkForOverrides( if (overriddenAccessors.length === 0) return; throw new LibraryOverrideError(url, overriddenAccessors); } + +/** + * This function is called only for ESM modules and generates a unique namespace for the module. + * @param url + * @param takenAccessors + * @param takenNamesMap + * @returns + */ +function generateUniqueAccessor( + url: string, + takenAccessors: Array, + takenNamesMap: Record, +) { + // extract file name from url + const urlObject = new URL(url); + // URL pattern for ESM modules from jsDelivr - https://cdn.jsdelivr.net/npm/stripe@13.3.0/+esm + // Assuming the file name is the last part of the path + const urlPathParts = urlObject.pathname.split("/"); + let fileName = urlPathParts.pop(); + fileName = fileName?.includes("esm") ? urlPathParts.pop() : fileName; + + // This should never happen. This is just to avoid the typescript error. + if (!fileName) throw new Error("Unable to generate a unique accessor"); + // Replace all non-alphabetic characters with underscores and remove trailing underscores + const validVar = fileName.replace(/[^a-zA-Z]/g, "_").replace(/_+$/, ""); + if ( + !takenAccessors.includes(validVar) && + !takenNamesMap.hasOwnProperty(validVar) + ) { + return validVar; + } + const index = 1; + while (true && index < 100) { + const name = `Library_${index}`; + if (!takenAccessors.includes(name) && !takenNamesMap.hasOwnProperty(name)) { + return name; + } + } + throw new Error("Unable to generate a unique accessor"); +} + +function flattenModule(module: Record) { + const keys = Object.keys(module); + // If there are no keys other than default, return default. + if (keys.length === 1 && keys[0] === "default") return module.default; + // If there are keys other than default, return a new object with all the keys + // and set its prototype of default export. + const libModule = Object.create(module.default); + for (const key of Object.keys(module)) { + if (key === "default") continue; + libModule[key] = module[key]; + } + return libModule; +}