Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: switch to yarn 3 + dedupe dependencies #20964

Merged
merged 18 commits into from
Mar 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/client-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ jobs:
run: |
PGPASSWORD='${{secrets.APPSMITH_PERFORMANCE_DB_PASSWORD}}' psql -h '${{secrets.APPSMITH_PERFORMANCE_DB_HOST}}' \
-U aforce_admin -d perf-infra -c \
"INSERT INTO public.run_meta (repo, gh_run_id, gh_run_attempt, is_active)
"INSERT INTO public.run_meta (repo, gh_run_id, gh_run_attempt, is_active)
VALUES ('${{github.repository}}', '${{github.run_id}}', '${{github.run_attempt}}', FALSE)"

# In case this is second attempt try restoring status of the prior attempt from cache
Expand All @@ -89,12 +89,12 @@ jobs:
with:
node-version: "16.14.0"
cache: "yarn"
cache-dependency-path: "app/client/yarn.lock"
cache-dependency-path: "app/yarn.lock"

# Install all the dependencies
- name: Install dependencies
if: steps.run_result.outputs.run_result != 'success'
run: yarn install --frozen-lockfile
run: yarn install --immutable

- name: Set the build environment based on the branch
if: steps.run_result.outputs.run_result != 'success'
Expand Down
6 changes: 2 additions & 4 deletions .github/workflows/rts-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,7 @@ jobs:
with:
node-version: "16.14.0"
cache: "yarn"
cache-dependency-path: "app/rts/yarn.lock"


cache-dependency-path: "app/yarn.lock"

# Here, the GITHUB_REF is of type /refs/head/<branch_name>. We extract branch_name from this by removing the
# first 11 characters. This can be used to build images for several branches
Expand All @@ -107,7 +105,7 @@ jobs:
# Install all the dependencies
- name: Install dependencies
if: steps.run_result.outputs.run_result != 'success'
run: yarn install --frozen-lockfile
run: yarn install --immutable

# Run the Jest tests only if the workflow has been invoked in a PR
- name: Run the jest tests
Expand Down
21 changes: 13 additions & 8 deletions .github/workflows/shared-modules.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ defaults:
run:
working-directory: app/shared


jobs:
test:
runs-on: ubuntu-latest
Expand All @@ -32,9 +31,8 @@ jobs:
github.event_name == 'push' ||
github.event_name == 'workflow_dispatch' ||
github.event_name == 'repository_dispatch'

steps:

steps:
# Check out merge commit with the base branch in case this workflow is invoked via pull request
- name: Checkout the merged commit from PR and base branch
if: inputs.pr != 0
Expand All @@ -49,7 +47,7 @@ jobs:
uses: actions/checkout@v3
with:
fetch-depth: 0

# Logging some params
- name: Figure out the PR number
run: echo ${{ inputs.pr }}
Expand All @@ -68,7 +66,7 @@ jobs:
path: |
~/run_result
key: ${{ github.run_id }}-${{ github.job }}-shared-modules

# Fetch prior run result
- name: Get the previous run result
id: run_result
Expand All @@ -83,12 +81,19 @@ jobs:
uses: actions/setup-node@v3
with:
node-version: "16.14.0"
cache: "yarn"
cache-dependency-path: "app/yarn.lock"

# Install all the dependencies
- name: Install dependencies
if: steps.run_result.outputs.run_result != 'success'
run: yarn install --immutable

# Runs the test cases
- name: Run the jest tests
if: steps.run_result.outputs.run_result != 'success'
run: REACT_APP_ENVIRONMENT=${{steps.vars.outputs.REACT_APP_ENVIRONMENT}} yarn run test:unit
run: REACT_APP_ENVIRONMENT=${{steps.vars.outputs.REACT_APP_ENVIRONMENT}} yarn workspaces foreach --include '@shared/*' run test:unit

# Set status = success
- name: Save the status of the run
run: echo "run_result=success" >> $GITHUB_OUTPUT > ~/run_result
run: echo "run_result=success" >> $GITHUB_OUTPUT > ~/run_result
7 changes: 7 additions & 0 deletions app/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
.pnp.*
.yarn/*
!.yarn/patches
!.yarn/plugins
!.yarn/releases
!.yarn/sdks
!.yarn/versions
13 changes: 13 additions & 0 deletions app/.yarn/patches/@blueprintjs-core-npm-3.47.0-a5bc1ea927.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
diff --git a/lib/esm/components/popover/popover.js b/lib/esm/components/popover/popover.js
index cb7faf374a596a7a32d5a3edc55445990bfbcfe1..b09c2e1fc72c9770cc4a05071c416ed898ff9cdb 100644
--- a/lib/esm/components/popover/popover.js
+++ b/lib/esm/components/popover/popover.js
@@ -167,6 +167,8 @@ var Popover = /** @class */ (function (_super) {
// close the popover if necessary.
if (e.relatedTarget != null && !_this.isElementInPopover(e.relatedTarget)) {
_this.handleMouseLeave(e);
+ } else if(e.relatedTarget === null) {
+ _this.handleMouseLeave(e);
}
}
_this.lostFocusOnSamePage = e.relatedTarget != null;
28 changes: 28 additions & 0 deletions app/.yarn/plugins/@yarnpkg/plugin-workspace-tools.cjs

Large diffs are not rendered by default.

22 changes: 22 additions & 0 deletions app/.yarn/plugins/plugin-dedupe-on-install.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
module.exports = {
name: `plugin-dedupe-on-install`,
factory: (require) => {
const {execute} = require('@yarnpkg/shell')
return {
hooks: {
// yarn / yarn install / yarn add / yarn dedupe -> afterAllInstalled
async afterAllInstalled() {
// use env var to prevent infinite loops
if (!process.env.DEDUPED && !process.argv.includes('dedupe')) {
process.env.DEDUPED = 'yes'
// fast check for duplicates
if (await execute('yarn dedupe --check')) {
// run actual dedupe/link step
await execute('yarn dedupe')
}
}
},
},
}
},
}
873 changes: 873 additions & 0 deletions app/.yarn/releases/yarn-3.4.1.cjs

Large diffs are not rendered by default.

9 changes: 9 additions & 0 deletions app/.yarnrc.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
nodeLinker: node-modules

plugins:
- path: .yarn/plugins/plugin-dedupe-on-install.cjs
spec: "https://raw.githubusercontent.com/ambar/yarn-plugin-dedupe-on-install/main/index.js"
- path: .yarn/plugins/@yarnpkg/plugin-workspace-tools.cjs
spec: "@yarnpkg/plugin-workspace-tools"

yarnPath: .yarn/releases/yarn-3.4.1.cjs
14 changes: 14 additions & 0 deletions app/client/craco.common.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,5 +72,19 @@ module.exports = {
css: false,
},
},
{
plugin: {
SatishGandham marked this conversation as resolved.
Show resolved Hide resolved
// Prioritize the local src directory over node_modules.
// This matters for cases where `src/<dirname>` and `node_modules/<dirname>` both exist –
// e.g., when `<dirname>` is `entities`: https://github.com/appsmithorg/appsmith/pull/20964#discussion_r1124782356
overrideWebpackConfig: ({ webpackConfig }) => {
webpackConfig.resolve.modules = [
path.resolve(__dirname, "src"),
...webpackConfig.resolve.modules,
];
return webpackConfig;
},
},
},
],
};
4 changes: 2 additions & 2 deletions app/client/cypress/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ if [ "$target" == "ci" ]; then
# On the CI server run the tests in parallel
# This requires the projectId and the record_key to be configured in your environment variables. By default this is defined on the CI server
echo "Got the Build ID: $BUILD_ID"
$(npm bin)/cypress run --headless \
yarn cypress run --headless \
--record --key "$CYPRESS_RECORD_KEY" --ci-build-id $BUILD_ID \
--parallel --group "Electrons on Gitlab CI" \
--spec "cypress/integration/Regression_TestSuite/**/*.js"
else
$(npm bin)/cypress run --headless --browser chromium --spec "cypress/integration/Regression_TestSuite/**/*.js"
yarn cypress run --headless --browser chromium --spec "cypress/integration/Regression_TestSuite/**/*.js"
fi
5 changes: 3 additions & 2 deletions app/client/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ module.exports = {
"^!!raw-loader!": "<rootDir>/test/__mocks__/derivedMock.js",
"test/(.*)": "<rootDir>/test/$1",
"@appsmith/(.*)": "<rootDir>/src/ee/$1",
"design-system-old": "<rootDir>/node_modules/design-system-old/build",
"^proxy-memoize$": "<rootDir>/node_modules/proxy-memoize/dist/wrapper.cjs",
"design-system-old": "<rootDir>/../node_modules/design-system-old/build",
"^proxy-memoize$":
"<rootDir>/../node_modules/proxy-memoize/dist/wrapper.cjs",
},
globals: {
"ts-jest": {
Expand Down
30 changes: 6 additions & 24 deletions app/client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,14 @@
"node": "^16.14.0",
"npm": "^8.5.5"
},
"workspaces": [
"packages/*"
],
"cracoConfig": "craco.dev.config.js",
"dependencies": {
"@blueprintjs/core": "^3.36.0",
"@blueprintjs/core": "^3.43.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

"@blueprintjs/datetime": "^3.23.6",
"@blueprintjs/icons": "^3.10.0",
"@blueprintjs/popover2": "^0.5.0",
"@blueprintjs/select": "^3.10.0",
"@design-system/wds": "*",
"@draft-js-plugins/editor": "^4.1.0",
"@draft-js-plugins/mention": "^4.5.1",
"@fusioncharts/powercharts": "^3.16.0",
Expand All @@ -38,7 +36,6 @@
"@uppy/react": "^1.11.2",
"@uppy/url": "^1.5.16",
"@uppy/webcam": "^1.8.4",
"@design-system/wds": "*",
"@welldone-software/why-did-you-render": "^4.2.5",
"acorn-walk": "^8.2.0",
"algoliasearch": "^4.2.0",
Expand Down Expand Up @@ -186,12 +183,10 @@
"eject": "react-scripts eject",
"start-prod": "REACT_APP_ENVIRONMENT=PRODUCTION craco start",
"cytest": "REACT_APP_TESTING=TESTING REACT_APP_ENVIRONMENT=DEVELOPMENT craco start & ./node_modules/.bin/cypress open",
"test:unit": "$(npm bin)/jest -b --colors --no-cache --silent --coverage --collectCoverage=true --coverageDirectory='../../' --coverageReporters='json-summary'",
"test:jest": "$(npm bin)/jest --watch",
"test:unit": "jest -b --colors --no-cache --silent --coverage --collectCoverage=true --coverageDirectory='../../' --coverageReporters='json-summary'",
"test:jest": "jest --watch",
"generate:widget": "plop --plopfile generators/index.js",
"postinstall": "patch-package && CURRENT_SCOPE=client node ../shared/install-dependencies.js",
"preinstall": "CURRENT_SCOPE=client node ../shared/build-shared-dep.js",
"install": "node cypress/apply-patches.js",
"postinstall": "node cypress/apply-patches.js",
"storybook": "yarn workspace @design-system/storybook storybook"
},
"browserslist": [
Expand Down Expand Up @@ -279,14 +274,14 @@
"eslint-plugin-react-hooks": "^2.3.0",
"eslint-plugin-sort-destructure-keys": "^1.3.5",
"factory.ts": "^0.5.1",
"jest": "^27.4.1",
"jest-canvas-mock": "^2.3.1",
"jest-styled-components": "^7.0.8",
"mocha": "^9.0.2",
"mocha-junit-reporter": "^2.0.0",
"mochawesome": "^7.1.2",
"mochawesome-report-generator": "^6.1.1",
"msw": "^0.28.0",
"patch-package": "^6.4.7",
"plop": "^3.1.1",
"postinstall-postinstall": "^2.1.0",
"prop-types": "^15.8.1",
Expand All @@ -307,18 +302,5 @@
"hooks": {
"pre-commit": "lint-staged && git-secrets --scan --untracked && git-secrets --scan -r"
}
},
"resolutions": {
SatishGandham marked this conversation as resolved.
Show resolved Hide resolved
"browserslist": "4.20.3",
"chokidar": "3.5.3",
"css-select": "4.1.3",
"ejs": "3.1.7",
"fast-csv": "4.3.6",
"focus-trap-react/**/tabbable": "5.2.1",
"json-schema": "0.4.0",
"node-fetch": "2.6.7",
"minimatch": "^5.0.0",
"loader-utils": "^2.0.4",
"json5": "2.2.3"
}
}
35 changes: 0 additions & 35 deletions app/client/patches/@blueprintjs+core+3.36.0.patch

This file was deleted.

3 changes: 2 additions & 1 deletion app/client/src/components/editorComponents/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
IconName,
MaybeElement,
IButtonProps,
IAnchorButtonProps,
} from "@blueprintjs/core";
import { Direction, Directions } from "utils/helpers";
import { omit } from "lodash";
Expand Down Expand Up @@ -71,7 +72,7 @@ const StyledButton = styled((props: IButtonProps & Partial<ButtonProps>) => (
${buttonStyles}
`;
const StyledAnchorButton = styled(
(props: IButtonProps & Partial<ButtonProps>) => (
(props: IAnchorButtonProps & Partial<ButtonProps>) => (
<BlueprintAnchorButton
{...omit(props, ["iconAlignment", "fluid", "filled", "outline"])}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ export type EditorConfig = {
mode: EditorModes;
tabBehaviour: TabBehaviour;
size: EditorSize;
hinting: Array<HintHelper>;
marking: Array<MarkHelper>;
hinting?: Array<HintHelper>;
marking?: Array<MarkHelper>;
folding?: boolean;
};

Expand Down
12 changes: 8 additions & 4 deletions app/client/src/components/editorComponents/CodeEditor/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -395,13 +395,15 @@ class CodeEditor extends Component<Props, State> {

CodeEditor.updateMarkings(
editor,
this.props.marking,
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
this.props.marking!, // ! since defaultProps are set
this.props.entitiesForNavigation,
);

this.hinters = CodeEditor.startAutocomplete(
editor,
this.props.hinting,
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
this.props.hinting!, // ! since defaultProps are set
this.props.dynamicData,
);

Expand Down Expand Up @@ -512,7 +514,8 @@ class CodeEditor extends Component<Props, State> {

CodeEditor.updateMarkings(
this.editor,
this.props.marking,
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
this.props.marking!, // ! since defaultProps are set
this.props.entitiesForNavigation,
);
});
Expand Down Expand Up @@ -967,7 +970,8 @@ class CodeEditor extends Component<Props, State> {
if (this.editor) {
CodeEditor.updateMarkings(
this.editor,
this.props.marking,
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
this.props.marking!, // ! since defaultProps are set
this.props.entitiesForNavigation,
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ class EmbeddedDatasourcePathComponent extends React.Component<

handleDatasourceHighlight = () => {
const { datasource } = this.props;
const authType = get(
const authType: string = get(
datasource,
"datasourceConfiguration.authentication.authenticationType",
"",
Expand Down
Loading