feat: Bundle optimization and first load improvements #21667

SatishGandham · 2023-03-22T05:22:05Z

Description

This is an intermediate branch for the first load optimization

TL;DR

Improves the first load times via bundle optimization, code splitting, and prioritizing delivery of critical code.

Change 1

The Evaluation and Tern workers have several large dependencies:

linkedom/worker
node-forge
tern JSON definitions
This PR removes them from the main bundle. When applied to appsmithorg/appsmith on top of perf: mark design system modules as side-effect-free design-system#389, this makes the bundle 980 minified kB smaller (7.46 → 6.48 minified MB). perf: move worker dependencies to the corresponding workers #20972

Change 2

switches the build target to modern browsers; this means stuff like class, async/await, etc is not transpiled anymore
code-splits away the code editor + sets up ESLint rules to enforce that
code-splits away Lottie + sets up ESLint rules to enforce that

In total, this saves 1.34 minified MBs (8.34 → 7 MB), getting rid of these modules (+ making others smaller, thanks to the modern build target):

Fixes # (issue)

if no issue exists, please create an issue and ask the maintainers about this first

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

First load optimization and some housekeeping stuff.

Test Plan

Test icons in icon picker and rest of the application. Check if icons are loading properly and they don't have intermedite blank state.
Test code editor how it loads are there any delays in interacting with it, use network throttling when testing.

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

This doesn’t work yet: postinstall scripts cause a recursive install loop

For some reason, after the Yarn v3 upgrade, some previously omitted TS errors are now visible. This commit fixes them.

During `yarn dedupe`, Yarn settled down on Blueprint Core v3.43. However, the current version of @blueprintjs/datetime relies on an import from @blueprintjs/core that’s not present in v3.43. In Yarn 1 setup, this used to work due to an accident: @blueprintjs/datetime installed a newer (3.47) version of @blueprintjs/core, which had the necessary import. Now that we deduped the versions, this doesn’t work anymore. This commit fixes this by upgrading the Blueprint Core to v3.47.

We moved package.json one level higher, and that somehow made webpack preference node_modules/entities over src/entities. The previous behavior was weird. (I can’t figure out *why* webpack would prefer src/entities, as it wasn’t configured [1] to do so.) But this commit fixes that. [1] https://github.com/facebook/create-react-app/blob/d960b9e38c062584ff6cfb1a70e1512509a966e7/packages/react-scripts/config/webpack.config.js#L306-L312

This: - adds `rts` into the list of workspaces - changes the build steps for `rts` to copy both the global and the workspaces’ node_modules into `dist`

This commit: - updates the icon names in the IconSelectControl tests. This was done because with the @blueprintjs/icon update, the control now has more icons, so the previous className checks don’t work anymore (now: https://user-images.githubusercontent.com/2953267/222846934-3b8ad6ae-8cf5-4de4-b5d0-cd3b7ef4234e.png; before: https://user-images.githubusercontent.com/2953267/222846936-bae05b25-dfc6-47cc-934d-3c1cb0d4080d.png) - switches the tests to snapshots: this makes it much easier to update icon names in case of future @blueprintjs/icon upgrades

…arn-3

perf: switch to yarn 3 + dedupe dependencies

Co-authored-by: Satish Gandham <[email protected]>

SatishGandham · 2023-03-22T09:31:48Z

/perf-test

github-actions · 2023-03-22T09:32:13Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/4488351778.
Workflow: Performance Tests Workflow.
Commit: ``.
PR: 21667.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=21667&runId=4488351778_1

…apps (#21602)

SatishGandham · 2023-03-23T10:03:26Z

/ok-to-test

github-actions · 2023-03-23T10:03:59Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/4499321176.
Workflow: Appsmith External Integration Test Workflow.
Commit: ``.
PR: 21667.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=21667&runId=4499321176_1

somangshu · 2023-05-10T08:52:36Z

/ok-to-test sha=f3e0d4f

github-actions · 2023-05-10T08:53:00Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/4935045082.
Workflow: Appsmith External Integration Test Workflow.
Commit: f3e0d4f.
PR: 21667.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=21667&runId=4935045082_1

somangshu · 2023-05-10T09:01:31Z

/build-deploy-preview env=release

github-actions · 2023-05-10T09:01:53Z

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/4935121390.
Workflow: On demand build Docker image and deploy preview.
skip-tests: ``.
env: release.
PR: 21667.
recreate: .

somangshu · 2023-05-10T09:31:57Z

/build-deploy-preview

github-actions · 2023-05-10T09:32:23Z

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/4935418138.
Workflow: On demand build Docker image and deploy preview.
skip-tests: . env: .
PR: 21667.
recreate: .

somangshu · 2023-05-10T11:15:18Z

Build running here

For some reason, the CI server fails to intercept JS requests. This change attempts to solve that

For more info why this is needed, see 5e5bc35

## Description #22652 introduced new styles to code in Appsmith. Along with it was some unintentional changes. Bundle optimization [PR](#21667) introduced, CodeEditorFallback component which incorrectly renders borders and causes UI to flicker. It also broke styles of lint message popover. This PR fixes these issues. Code changes - Added 6px padding to code editor lines - Use class variables instead of ref - Removed border for JSEditor's CodeEditorFallback component - Fixes lint message style > > Links to Notion, Figma or any other documents that might be relevant to the PR > https://www.notion.so/appsmith/Visual-changes-to-code-in-Appsmith-8bb530c60ee844e5b44adec039bf9280#d99e8450f022439f845b5a0d7deb1d3f > #### PR fixes following issue(s) Fixes # (issue number) > if no issue exists, please create an issue and ask the maintainers about this first > > #### 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 - Bug fix (non-breaking change which fixes an issue) > > > ## Testing > #### How Has This Been Tested? - [x] Manual > > #### Test Plan Visual checks to test for changes as per [this](https://www.notion.so/appsmith/Error-tooltip-for-data-fields-on-property-pane-Needs-design-810cdc489c3e4168a0b1b348730ac98c?utm_content=810cdc48-9c3e-4168-a0b1-b348730ac98c&utm_campaign=TGA9SH07N&n=slack&n=slack_link_unfurl&pvs=6) Notion doc > #### Issues raised during DP testing None > ## 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 - [x] 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: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Test-plan-implementation#speedbreaker-features-to-consider-for-every-change) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans/_edit#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: Ravi Kumar Prasad <[email protected]>

iamenrique · 2023-05-17T17:57:21Z

After merging this PR, is everything in this guide still valid? https://github.com/appsmithorg/appsmith/blob/release/contributions/ClientSetup.md

I noticed node_modules and ESLint configuration are now in a different level. Does this mean the yarn install has to be done in the app/ folder rather than app/client? Also had to change the ESLint configuration manually in IntelliJ (was automatic before).

Cc. @riodeuno

SatishGandham · 2023-05-17T17:59:54Z

You can do yarn install in any folder under app.

We will move all UI related code under client in a couple of weeks.

iamenrique · 2023-05-17T18:02:09Z

@SatishGandham Understood. Is there any issue number / branch / PR I can follow to be aware of those updates? Thanks in advance.

SatishGandham · 2023-05-18T09:34:27Z

@iamenrique , #23395

## Description This issue is happening because we are getting lodash library as `undefined` when we pass it to` derived.value` function here. This is happening because of the following reason: * [Bundle optimisation PR](#21667) introduces a new plugin called: `babel-plugin-lodash`. It helps in transforming the imports. * But this plugin doesn't work on the following pattern: `import _ from "lodash"`. Here it replaces `_` with `undefined` [ This file](https://github.com/appsmithorg/appsmith/blob/3bc50d9632cf269935aed6657092ddb8b7e8c92f/app/client/src/workers/common/JSLibrary/lodash-wrapper.js#L1) explains the exact scenario that we are facing. There is also [an open bug](lodash/babel-plugin-lodash#235) for this issue on the `babel-plugin-lodash` repository. This PR imports lodash from the lodash-wrapper instead of directly importing as follows: `import _ from "lodash"` > #### PR fixes following issue(s) Fixes #23671 #### Type of change - Bug fix (non-breaking change which fixes an issue) ## Testing > #### How Has This Been Tested? - [x] Manual - [ ] Jest - [x] Cypress - should check that value entered in currency field appears in the actual field > > #### 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 - [ ] 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: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] 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 - [ ] 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

## Description This issue is happening because we are getting lodash library as `undefined` when we pass it to` derived.value` function here. This is happening because of the following reason: * [Bundle optimisation PR](#21667) introduces a new plugin called: `babel-plugin-lodash`. It helps in transforming the imports. * But this plugin doesn't work on the following pattern: `import _ from "lodash"`. Here it replaces `_` with `undefined` [ This file](https://github.com/appsmithorg/appsmith/blob/3bc50d9632cf269935aed6657092ddb8b7e8c92f/app/client/src/workers/common/JSLibrary/lodash-wrapper.js#L1) explains the exact scenario that we are facing. There is also [an open bug](lodash/babel-plugin-lodash#235) for this issue on the `babel-plugin-lodash` repository. This PR imports lodash from the lodash-wrapper instead of directly importing as follows: `import _ from "lodash"` > #### PR fixes following issue(s) Fixes #23671 #### Type of change - Bug fix (non-breaking change which fixes an issue) ## Testing > #### How Has This Been Tested? - [x] Manual - [ ] Jest - [x] Cypress - should check that value entered in currency field appears in the actual field > > #### 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 - [ ] 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: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] 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 - [ ] 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 (cherry picked from commit f47b1c8)

iamakulov and others added 24 commits February 26, 2023 04:22

Yarn v3, step 1: switch to Yarn workspaces

e35a6fb

This doesn’t work yet: postinstall scripts cause a recursive install loop

Yarn v3, step 2: upgrade to Yarn 3

2209c39

Yarn v3, step 3: make shared/ast work with workspaces

f6928f8

Yarn v3, step 4: migrate to native package patching

2ab3974

Dedupe packages

dccf98a

Enable autodeduping on install

af8033c

Yarn v3: fix surfaced TS errors

08ad0f8

For some reason, after the Yarn v3 upgrade, some previously omitted TS errors are now visible. This commit fixes them.

Yarn v3: make Cypress work with Yarn

01e7b3e

Elaborate on a different resolve.modules config

9924fd8

package.jsons: drop $(npm bin) calls

d06877c

app/rts: switch to Yarn workspaces

b6a347b

This: - adds `rts` into the list of workspaces - changes the build steps for `rts` to copy both the global and the workspaces’ node_modules into `dist`

Fix CI jobs

0372ff9

Fix Jest tests in app/client

9093005

Merge remote-tracking branch 'origin/bundle-optimization' into perf/y…

8ef6d9b

…arn-3

Fix TypeScript issues surfaced after the _.get types update

8bc4a45

Merge pull request #20964 from 3perf/perf/yarn-3

497412a

perf: switch to yarn 3 + dedupe dependencies

Merge branch 'release' into bundle-optimization

298aaf4

Fix an incorrect import

30ee6d7

fix: yarn 3 upgrade issues (#21269)

e1dcb05

perf: move worker dependencies to the corresponding workers (#20972)

99321b1

perf: code-split svg, remix and blueprint icons away (#21438)

b319a03

Co-authored-by: Satish Gandham <[email protected]>

perf: code-split codemirror & lottie + optimize the build for modern …

f8a2c76

…apps (#21602)

Merge branch 'release' into bundle-optimization

efe5b87

Merge branch 'release' into bundle-optimization

f3e0d4f

Satish Gandham and others added 7 commits May 10, 2023 18:31

Merge branch 'release' into bundle-optimization

e3bd8f2

Merge branch 'release' into bundle-optimization

c4968a4

link rel spec: disable caching completely + make matching more forgiving

dc1fcba

For some reason, the CI server fails to intercept JS requests. This change attempts to solve that

Fix a regression with a missing icon attribute on Blueprint icons

cb0c158

Merge remote-tracking branch 'origin/release' into bundle-optimization

09dc196

Postgres spec: delete items based on a dynamic list

9e06c94

For more info why this is needed, see 5e5bc35

Merge branch 'release' into bundle-optimization

6f629b0

SatishGandham merged commit 83538ad into release May 11, 2023

SatishGandham deleted the bundle-optimization branch May 11, 2023 05:26

SatishGandham restored the bundle-optimization branch May 13, 2023 07:06

arunvjn mentioned this pull request May 15, 2023

fix: UI issues with code editor #23332

Merged

17 tasks

ramsaptami mentioned this pull request May 16, 2023

[Feature]: Support autocomplete helper text to explain JS functions #23381

Closed

1 task

ashit-rath mentioned this pull request Jun 19, 2023

Lodash is undefined in derived.js of Currency Widget TypeError: Cannot read properties of undefined (reading 'isNaN') #23671

Closed

keyurparalkar mentioned this pull request Jun 19, 2023

fix: change to lodash wrapper #24610

Merged

19 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Bundle optimization and first load improvements #21667

feat: Bundle optimization and first load improvements #21667

SatishGandham commented Mar 22, 2023 •

edited

Loading

SatishGandham commented Mar 22, 2023

github-actions bot commented Mar 22, 2023

SatishGandham commented Mar 23, 2023

github-actions bot commented Mar 23, 2023

somangshu commented May 10, 2023

github-actions bot commented May 10, 2023

somangshu commented May 10, 2023

github-actions bot commented May 10, 2023

somangshu commented May 10, 2023

github-actions bot commented May 10, 2023

somangshu commented May 10, 2023

iamenrique commented May 17, 2023 •

edited

Loading

SatishGandham commented May 17, 2023

iamenrique commented May 17, 2023

SatishGandham commented May 18, 2023

feat: Bundle optimization and first load improvements #21667

feat: Bundle optimization and first load improvements #21667

Conversation

SatishGandham commented Mar 22, 2023 • edited Loading

Description

TL;DR

Change 1

Change 2

Type of change

Test Plan

Checklist:

Dev activity

QA activity:

SatishGandham commented Mar 22, 2023

github-actions bot commented Mar 22, 2023

SatishGandham commented Mar 23, 2023

github-actions bot commented Mar 23, 2023

somangshu commented May 10, 2023

github-actions bot commented May 10, 2023

somangshu commented May 10, 2023

github-actions bot commented May 10, 2023

somangshu commented May 10, 2023

github-actions bot commented May 10, 2023

somangshu commented May 10, 2023

iamenrique commented May 17, 2023 • edited Loading

SatishGandham commented May 17, 2023

iamenrique commented May 17, 2023

SatishGandham commented May 18, 2023

SatishGandham commented Mar 22, 2023 •

edited

Loading

iamenrique commented May 17, 2023 •

edited

Loading