From e5d89e7099e3dab70611b920a3a345f933f9b008 Mon Sep 17 00:00:00 2001 From: Favour Ohanekwu Date: Wed, 27 Jul 2022 10:18:39 +0100 Subject: [PATCH] chore: Upgrade astring version from v1.7.5 to v1.8.3 (#15245) ## Description Previous version of the astring library doesn't preserve parenthesis when transforming AST to JS Code. This led to syntax error when null coalesce and logical expressions are used together. The latest version of astring library fixes this. screenshot_2022-07-16_at_04 29 52 Fixes #15283 ## Type of change - Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? Test plan - [x] https://github.com/appsmithorg/TestSmith/issues/1970 ## Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes --- .../ClientSideTests/BugTests/Bug15283_spec.ts | 107 ++++++++++++++++++ .../Linting/ErrorReporting_spec.ts | 10 +- app/client/src/workers/ast.ts | 1 - app/client/yarn.lock | 5 +- 4 files changed, 113 insertions(+), 10 deletions(-) create mode 100644 app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/BugTests/Bug15283_spec.ts diff --git a/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/BugTests/Bug15283_spec.ts b/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/BugTests/Bug15283_spec.ts new file mode 100644 index 0000000000..fd51968471 --- /dev/null +++ b/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/BugTests/Bug15283_spec.ts @@ -0,0 +1,107 @@ +import { ObjectsRegistry } from "../../../../support/Objects/Registry"; + +const jsEditor = ObjectsRegistry.JSEditor, + ee = ObjectsRegistry.EntityExplorer, + locator = ObjectsRegistry.CommonLocators, + agHelper = ObjectsRegistry.AggregateHelper; + +const assertLintErrorAndOutput = ( + code: string, + hasLintError: boolean, + output?: string, +) => { + jsEditor.EditJSObj(code); + // Wait for parsing to be complete + agHelper.Sleep(3000); + + hasLintError + ? agHelper.AssertElementExist(locator._lintErrorElement) + : agHelper.AssertElementAbsence(locator._lintErrorElement); + + if (output) { + agHelper.GetNClick(jsEditor._runButton); + cy.contains( + output === "undefined" ? "did not return any data" : output, + ).should("exist"); + } +}; + +describe("Correctly parses JS Function", () => { + before(() => { + ee.DragDropWidgetNVerify("singleselecttreewidget", 300, 500); + }); + it("1. Preserves parenthesis in user's code", () => { + const JS_OBJECT_BODY = `export default{ + labels: { + filterText: "Expected result" + }, + testFun: (searchText)=>{ + const filterText = searchText ?? (this.labels?.filterText + "s" || ''); + return filterText; + } + } + `; + jsEditor.CreateJSObject(JS_OBJECT_BODY, { + paste: true, + completeReplace: true, + toRun: false, + shouldCreateNewJSObj: true, + }); + + // confirm there is no parse error + jsEditor.AssertParseError(false, false); + // Wait for parsing to be complete + agHelper.Sleep(2000); + // run + agHelper.GetNClick(jsEditor._runButton); + + // confirm there is no function execution error + jsEditor.AssertParseError(false, false); + + cy.contains("Expected results").should("exist"); + }); + it("2. TC 1970 - Outputs expected result", () => { + const getJSObjectBody = (expression: string) => `export default{ + myFun1: ()=>{ + const result = ${expression}; + return result; + } + } + `; + const expression1 = `null ?? (TreeSelect1.selectedOptionLabel || undefined)`; + const expression2 = `null ?? (TreeSelect1.selectedOptionLabel && undefined)`; + const expression3 = `!null ?? (!TreeSelect1.selectedOptionLabel || !undefined)`; + const expression4 = `null ?? (TreeSelect1.selectedOptionLabel.unknown || undefined)`; + const expression5 = `null ?? (null || TreeSelect1.selectedOptionLabel + " hi")`; + const expression6 = `null ?? (TreeSelect1.selectedOptionLabel || "hi")`; + const expression7 = `null ?? (!TreeSelect1.selectedOptionLabel + " that" || "hi")`; + const expression8 = `null ?? (TreeSelect1.selectedOptionLabel && "hi")`; + const expression9 = `null ?? (TreeSelect1.selectedOptionLabel && undefined)`; + const expression10 = `null ?? (!TreeSelect1.selectedOptionLabel && "hi")`; + const expression11 = `(null || !TreeSelect1.selectedOptionLabel) ?? TreeSelect1.selectedOptionLabel.unknown`; + const expression12 = `(null && !TreeSelect1.selectedOptionLabel) ?? "hi"`; + const expression13 = `(true || "universe") ?? "hi"`; + const expression14 = `null ?? TreeSelect1.selectedOptionLabel || undefined`; + const expression15 = `null ?? TreeSelect1.selectedOptionLabel && undefined`; + const expression16 = `!null ?? !TreeSelect1.selectedOptionLabel || !undefined`; + const expression17 = `null ?? TreeSelect1.selectedOptionLabel || undefined`; + + assertLintErrorAndOutput(getJSObjectBody(expression1), false, "B"); + assertLintErrorAndOutput(getJSObjectBody(expression2), false, "B"); + assertLintErrorAndOutput(getJSObjectBody(expression3), false, "true"); + assertLintErrorAndOutput(getJSObjectBody(expression4), false, "undefined"); + assertLintErrorAndOutput(getJSObjectBody(expression5), false, "B hi"); + assertLintErrorAndOutput(getJSObjectBody(expression6), false, "hi"); + assertLintErrorAndOutput(getJSObjectBody(expression7), false, "false that"); + assertLintErrorAndOutput(getJSObjectBody(expression8), false, "hi"); + assertLintErrorAndOutput(getJSObjectBody(expression9), false, "hi"); + assertLintErrorAndOutput(getJSObjectBody(expression10), false, "hi"); + assertLintErrorAndOutput(getJSObjectBody(expression11), false, "false"); + assertLintErrorAndOutput(getJSObjectBody(expression12), false, "hi"); + assertLintErrorAndOutput(getJSObjectBody(expression13), false, "true"); + assertLintErrorAndOutput(getJSObjectBody(expression14), true, undefined); + assertLintErrorAndOutput(getJSObjectBody(expression15), true, undefined); + assertLintErrorAndOutput(getJSObjectBody(expression16), true, undefined); + assertLintErrorAndOutput(getJSObjectBody(expression17), true, undefined); + }); +}); diff --git a/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/Linting/ErrorReporting_spec.ts b/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/Linting/ErrorReporting_spec.ts index e3c44c8e30..2d502df228 100644 --- a/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/Linting/ErrorReporting_spec.ts +++ b/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/Linting/ErrorReporting_spec.ts @@ -302,16 +302,12 @@ describe("Lint error reporting", () => { agHelper.AssertElementAbsence(locator._lintErrorElement); }); - function MouseHoverNVerify( - lintOn: string, - debugMsg: string, - isError = true, - ) { + function MouseHoverNVerify(lintOn: string, debugMsg: string, isError = true) { agHelper.Sleep(); - let element = isError + const element = isError ? cy.get(locator._lintErrorElement) : cy.get(locator._lintWarningElement); - element + element .contains(lintOn) .should("exist") .first() diff --git a/app/client/src/workers/ast.ts b/app/client/src/workers/ast.ts index d132b416a0..1b9a99bac4 100644 --- a/app/client/src/workers/ast.ts +++ b/app/client/src/workers/ast.ts @@ -384,7 +384,6 @@ export const parseJSObjectWithAST = ( } }, }); - JSObjectProperties.forEach((node) => { let params = new Set(); const propertyNode = node; diff --git a/app/client/yarn.lock b/app/client/yarn.lock index e945b22fc6..53d19673a8 100644 --- a/app/client/yarn.lock +++ b/app/client/yarn.lock @@ -4335,8 +4335,9 @@ astral-regex@^2.0.0: resolved "https://registry.npmjs.org/astral-regex/-/astral-regex-2.0.0.tgz" astring@^1.7.5: - version "1.7.5" - resolved "https://registry.npmjs.org/astring/-/astring-1.7.5.tgz" + version "1.8.3" + resolved "https://registry.yarnpkg.com/astring/-/astring-1.8.3.tgz#1a0ae738c7cc558f8e5ddc8e3120636f5cebcb85" + integrity sha512-sRpyiNrx2dEYIMmUXprS8nlpRg2Drs8m9ElX9vVEXaCB4XEAJhKfs7IcX0IwShjuOAjLR6wzIrgoptz1n19i1A== async-limiter@~1.0.0: version "1.0.1"