perf: switch to yarn 3 + dedupe dependencies #20964

iamakulov · 2023-02-26T04:16:21Z

Description

This PR:

upgrades Appsmith to Yarn v3
- see what’s new: https://yarnpkg.com/getting-started/qa#why-should-you-upgrade-to-yarn-modern
- this upgrade also makes it possible to enable Yarn Plug'n'Play, which will make Yarn commands even faster. I haven’t attempted this (it needs a bit of work) but I’d recommend trying it out: https://yarnpkg.com/features/pnp
migrates the shared/ast to Yarn Workspaces
- this makes managing shared packages much simpler (you don’t need the whole build-shared-dep.js, install-dependencies.js, etc logic)
- and makes Yarn runs cheaper (shared/ast is now rebuilt only when needed instead of on every yarn add)
replaces patch-package with Yarn’s native package patching
dedupes all packages
- (did you know we had 4 versions of @blueprintjs/core installed?)
makes yarn dedupe run on every install

Apart from DX improvements, this also makes the bundle almost 10% smaller (−870 kB, 9.01 → 8.14 MB minified).

Type of change

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

How Has This Been Tested?

Manual: smoke testing (clicking around the dashboard and the editor)

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

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.

welcome · 2023-02-26T04:16:24Z

Welcome to the Appsmith community! Thank you for your first pull request and making this project better. 🤗 Please make sure that you raise a review request so your code change does not go unnoticed.

vercel · 2023-02-26T04:16:25Z

@iamakulov is attempting to deploy a commit to the Appsmith Team on Vercel.

A member of the Team first needs to authorize it.

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

SatishGandham · 2023-02-28T08:57:24Z

/ok-to-test

github-actions · 2023-02-28T08:58:23Z

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

app/client/craco.common.config.js

app/client/package.json

app/client/patches/@blueprintjs+core+3.36.0.patch

yaldram · 2023-03-02T06:04:35Z

app/client/package.json

@@ -8,7 +8,7 @@
  },
  "cracoConfig": "craco.dev.config.js",
  "dependencies": {
-    "@blueprintjs/core": "^3.36.0",
+    "@blueprintjs/core": "^3.43.0",


Blueprint is updated here, might need to check if everything is working as expected.

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

app/.yarn/plugins/@yarnpkg/plugin-workspace-tools.cjs

…arn-3

This upgrades the build script to use Yarn 3 properly. This was missed while incorporating appsmithorg#20776 into appsmithorg#20964.

The package got upgraded during #20964. The reason we’re downgrading is we want to keep the list of icons we support fixed. If we ever have to migrate off @blueprintjs/icons, we’ll have to manually maintain all icons that are available in the package.

iamakulov added 8 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.

iamakulov force-pushed the perf/yarn-3 branch from 18006c3 to e249cab Compare February 26, 2023 04:55

iamakulov changed the title ~~Perf: Yarn 3 + deduping dependencies~~ perf: switch to yarn 3 + dedupe dependencies Feb 26, 2023

Yarn v3: make Cypress work with Yarn

01e7b3e

iamakulov mentioned this pull request Feb 27, 2023

perf: mark design system modules as side-effect-free appsmithorg/design-system#389

Merged

12 tasks

SatishGandham changed the base branch from release to bundle-optimization February 28, 2023 08:55

SatishGandham suggested changes Mar 2, 2023

View reviewed changes

app/client/craco.common.config.js Show resolved Hide resolved

app/client/package.json Show resolved Hide resolved

app/client/patches/@blueprintjs+core+3.36.0.patch Show resolved Hide resolved

SatishGandham requested a review from yaldram March 2, 2023 04:35

yaldram approved these changes Mar 2, 2023

View reviewed changes

iamakulov added 2 commits March 3, 2023 18:22

Elaborate on a different resolve.modules config

9924fd8

package.jsons: drop $(npm bin) calls

d06877c

iamakulov force-pushed the perf/yarn-3 branch 3 times, most recently from ea30c41 to 901056a Compare March 3, 2023 22:50

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`

iamakulov force-pushed the perf/yarn-3 branch from e33f2c4 to 7ff435f Compare March 4, 2023 00:44

iamakulov added 2 commits March 4, 2023 02:01

Fix CI jobs

0372ff9

Fix Jest tests in app/client

9093005

iamakulov force-pushed the perf/yarn-3 branch from 7ff435f to c5a956f Compare March 4, 2023 01:01

SatishGandham reviewed Mar 7, 2023

View reviewed changes

app/.yarn/plugins/@yarnpkg/plugin-workspace-tools.cjs Show resolved Hide resolved

iamakulov added 2 commits March 7, 2023 16:39

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

8ef6d9b

…arn-3

Fix TypeScript issues surfaced after the _.get types update

8bc4a45

SatishGandham merged commit 497412a into appsmithorg:bundle-optimization Mar 7, 2023

iamakulov mentioned this pull request Mar 8, 2023

fix: yarn 3 upgrade issues #21269

Merged

13 tasks

iamakulov added a commit to 3perf/appsmith-feb2023 that referenced this pull request Mar 20, 2023

Fix the infinite build

729f3b7

This upgrades the build script to use Yarn 3 properly. This was missed while incorporating appsmithorg#20776 into appsmithorg#20964.

iamakulov mentioned this pull request Jun 30, 2023

perf: reduce the bundle size, vol. 2 #24969

Merged

19 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf: switch to yarn 3 + dedupe dependencies #20964

perf: switch to yarn 3 + dedupe dependencies #20964

iamakulov commented Feb 26, 2023 •

edited

Loading

welcome bot commented Feb 26, 2023

vercel bot commented Feb 26, 2023

SatishGandham commented Feb 28, 2023

github-actions bot commented Feb 28, 2023

yaldram Mar 2, 2023

perf: switch to yarn 3 + dedupe dependencies #20964

perf: switch to yarn 3 + dedupe dependencies #20964

Conversation

iamakulov commented Feb 26, 2023 • edited Loading

Description

Type of change

How Has This Been Tested?

Test Plan

Issues raised during DP testing

Checklist:

Dev activity

QA activity:

welcome bot commented Feb 26, 2023

vercel bot commented Feb 26, 2023

SatishGandham commented Feb 28, 2023

github-actions bot commented Feb 28, 2023

yaldram Mar 2, 2023

Choose a reason for hiding this comment

iamakulov commented Feb 26, 2023 •

edited

Loading