Commit Graph

8 Commits

Author SHA1 Message Date
arunvjn
713ca6ffc4
chore: type check callback functions in setInterval and setTimeout before invoking. (#20592)
## Description
We have recently changed the implementation for setInterval in
https://github.com/appsmithorg/appsmith/pull/20168.
This has the potential to blow up our sentry quota if someone sets up an
empty setInterval function with no callback in it and execute it. This
PR adds a type check before invoking the callback to ensure that the
passed in callback is indeed, a function.

Fixes https://github.com/appsmithorg/appsmith/issues/20591

https://appsmith.sentry.io/issues/3928721303/?project=1546547&query=is%3Aunresolved+worker&referrer=issue-stream&statsPeriod=14d

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
- Chore (housekeeping or task changes that don't impact user perception)

## How Has This Been Tested?
- Manual

### Test Plan
> Add Testsmith test cases links that relate to this PR

### Issues raised during DP testing
> Link issues raised during DP testing for better visiblity and tracking
(copy link from comments dropped on this PR)


## 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
- [ ] 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
- [ ] PR is being merged under a feature flag


### QA activity:
- [ ] Test plan has been approved by relevant developers
- [ ] Test plan has been peer reviewed by QA
- [ ] Cypress test cases have been added and approved by either SDET or
manual QA
- [ ] Organized project review call with relevant stakeholders after
Round 1/2 of QA
- [ ] Added Test Plan Approved label after reveiwing all Cypress test
2023-02-16 16:45:01 +05:30
Rishabh Rathod
b644388023
chore: export batching method for ee PR (#20631)
## Description

Changes required for this PR
https://github.com/appsmithorg/appsmith-ee/pull/1083/

## Type of change

- Chore (housekeeping or task changes that don't impact user perception)

## How Has This Been Tested?


## Checklist:
### Dev activity
- [ ] My code follows the style guidelines of this project
- [ ] 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
- [ ] PR is being merged under a feature flag


### QA activity:
- [ ] Test plan has been approved by relevant developers
- [ ] Test plan has been peer reviewed by QA
- [ ] Cypress test cases have been added and approved by either SDET or
manual QA
- [ ] Organized project review call with relevant stakeholders after
Round 1/2 of QA
- [ ] Added Test Plan Approved label after reveiwing all Cypress test
2023-02-15 21:57:27 +05:30
arunvjn
1d9d5bb197
fix: access outer scope variables inside callbacks (#20168)
## Description
Any platform function that accepts a callback were unable to access the
variables declared in its parent scopes. This was a implementation miss
when we originally designed platform functions and again when we turned
almost every platform function into a Promise. This PR fixes this
limitation along with some other edge cases.

- Access outer scope variables inside the callback of run, postMessage,
setInterval, getGeoLocation and watchGeolocation functions.
- Fixes certain edge cases where functions with callbacks when called
inside the then block doesn't get executed. Eg `showAlert.then(() => /*
Doesn't execute */ Api1.run(() => {}))`
- Changes the implementation of all the platform function in appsmith to
maintain the execution metadata (info on from where a function was
invoked, event associated with it etc)

#### Refactor changes
- Added a new folder **_fns_** that would now hold all the platform
functions.
- Introduced a new ExecutionMetadata singleton class that is now
responsible for hold all the meta data related to the current
evaluation.
- Remove TRIGGER_COLLECTOR array where all callback based platform
functions were batched and introduced an Event Emitter based
implementation to handle batched fn calls.
- All callback based functions now emits event when invoked. These
events have handlers attached to the TriggerEmitter object. These
handler does the job of batching these invocations and telling the main
thread. It also ensures that platform fn calls that gets triggered out
the the context of a request/response cycle work.

#### Architecture
<img width="751" alt="Screenshot 2023-02-07 at 10 04 26"
src="https://user-images.githubusercontent.com/32433245/217259200-5eac71bc-f0d3-4d3c-9b69-2a8dc81351bc.png">

Fixes #13156
Fixes #20225 

## Type of change
- Bug fix (non-breaking change which fixes an issue)
- Refactor

## How Has This Been Tested?
- Jest
- Cypress
- Manual


### Test Plan
  - [ ] https://github.com/appsmithorg/TestSmith/issues/2181
  - [ ] https://github.com/appsmithorg/TestSmith/issues/2182
- [ ] Post message -
https://appsmith-git-chore-outer-scope-variable-access-get-appsmith.vercel.app/app/post-msg-app/page1-635fcfba2987b442a739b938/edit
- [ ] Apps:
https://appsmith-git-chore-outer-scope-variable-access-get-appsmith.vercel.app/app/earworm-1/home-630c9d85b4658d0f257c4987/edit
- [ ]
https://appsmith-git-chore-outer-scope-variable-access-get-appsmith.vercel.app/app/automation-test-cases/page-1-630c6b90d4ecd573f6bb01e9/edit#0hmn8m90ei

### Issues raised during DP testing
> Link issues raised during DP testing for better visiblity and tracking
(copy link from comments dropped on this PR)

## 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
- [x] 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:
- [ ] Test plan has been approved by relevant developers
- [ ] Test plan has been peer reviewed by QA
- [ ] Cypress test cases have been added and approved by either SDET or
manual QA
- [ ] Organized project review call with relevant stakeholders after
Round 1/2 of QA
- [ ] Added Test Plan Approved label after reviewing all Cypress test
2023-02-12 00:03:20 +05:30
Rishabh Rathod
cd051ac941
fix: Make async evaluation as default (#20095)
## Description

Current evaluation architecture has 2 natures of evaluation
- sync evaluation ( evaluation for data fields )
- async evaluation ( evaluation for trigger events )

For every evaluation, this nature needs to be set according to the field
being evaluated.
It was noticed that for async code evaluation where a code block
executes only after the resolution of promise, for example,

`fetch().then((res) => { showAlert("fetched") });`

Here, the `showAlert` only executes when the fetch is completed and
before that, it could be possible that we have switched to sync
evaluation in the worker. This would lead to a `showAlert` throwing
error.

Hence, we resolve this issue by making `async evaluation` as the default
nature of evaluation.

Fixes https://github.com/appsmithorg/appsmith/issues/19733

## Type of change


- Bug fix (non-breaking change which fixes an issue)


## How Has This Been Tested?


- Manual
1. Validated Supabse app to check the error and JSPDF 
2. Validated sync data field with sync function and framework function -
verify that error for async function is displayed properly
3. Validated older apps(Automation app/Silly string) to verify the Async
error is not displayed
4. .then and catch block in async function - 

- Jest
- Cypress

### Test Plan

### Issues raised during DP testing


---------

Co-authored-by: Aishwarya UR <aishwarya@appsmith.com>
2023-01-30 10:57:12 +05:30
albinAppsmith
110e6085b8
feat: Renamed design system package (#19854)
## Description

This PR includes changes for renaming design system package. Since we
are building new package for the refactored design system components,
the old package is renaming to design-system-old.

Fixes #19536 

## Type of change

- New feature (non-breaking change which adds functionality)
- Breaking change (fix or feature that would cause existing
functionality to not work as expected)


## How Has This Been Tested?

- Manual
- Jest
- Cypress

### Test Plan
> Add Testsmith test cases links that relate to this PR

### Issues raised during DP testing
> Link issues raised during DP testing for better visiblity and tracking
(copy link from comments dropped on this PR)


## Checklist:
### Dev activity
- [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
- [x] My changes generate no new warnings
- [ ] 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:
- [ ] Test plan has been approved by relevant developers
- [ ] Test plan has been peer reviewed by QA
- [ ] Cypress test cases have been added and approved by either SDET or
manual QA
- [ ] Organized project review call with relevant stakeholders after
Round 1/2 of QA
- [ ] Added Test Plan Approved label after reveiwing all Cypress test
2023-01-23 09:20:47 +05:30
arunvjn
e4a84b7c23
chore: support localstorage APIs using appsmith store functions (#19789)
## Description
Most JS libraries are written with the intent to run of browsers main
thread and not a web worker. JS libraries relies on the support for
local storage APIs fail because web worker do not have access to local
storage APIs. Examples of libraries that rely on localStorage include
Mixpanel-browser, Supabasev2 etc.

- Mocks localStorage API by using the respective store operation under
the hood.
- Autocomplete will not suggest localStorage APIs to promote the use of
appsmith store functions.

Fixes #19792 

### Type of change
- Chore (housekeeping or task changes that don't impact user perception)

### How Has This Been Tested?
- Manual
1. Instal supobase -
https://cdn.jsdelivr.net/npm/@supabase/supabase-js@2.4.0/dist/umd/supabase.min.js
and verify that its working properly
2. Instal recommend libraries and verify that it is getting installed
properly


> Add Testsmith test cases links that relate to this PR

### Issues raised during DP testing
> Link issues raised during DP testing for better visiblity and tracking
(copy link from comments dropped on this PR)


## 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
- [x] 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:
- [ ] Test plan has been approved by relevant developers
- [ ] Test plan has been peer reviewed by QA
- [ ] Cypress test cases have been added and approved by either SDET or
manual QA
- [ ] Organized project review call with relevant stakeholders after
Round 1/2 of QA
- [ ] Added Test Plan Approved label after reveiwing all Cypress test
2023-01-19 10:01:26 +05:30
Rishabh Rathod
ea5a397f50
chore: Refactor for JSFunction execution auditLogs (#19671)
## Description

As auditLog related Code refactor for EE auditLog changes.

- Moving evalWorkerActionSaga to new file
- Created JSFunctionExecutionSaga & postJSFunctionExecutionLog
   - logic for the functionality of it would be added in EE.
- Made required changes in the JSProxy file to send functionName as a
parameter.
## Type of change

Code refactor


## How Has This Been Tested?

- Manual
- Jest
- Cypress


## Checklist:
### Dev activity
- [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
- [x] My changes generate no new warnings
- [ ] 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:
- [ ] Test plan has been approved by relevant developers
- [ ] Test plan has been peer reviewed by QA
- [ ] Cypress test cases have been added and approved by either SDET or
manual QA
- [ ] Organized project review call with relevant stakeholders after
Round 1/2 of QA
- [ ] Added Test Plan Approved label after reveiwing all Cypress test
2023-01-16 17:26:18 +05:30
arunvjn
c8063743a2
fix: Batch operations on appsmith store for performance gains (#19247) 2023-01-10 10:23:08 +05:30