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.

Sign up for GitHub

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

Jump to bottom

Upgrade yarn to v3.8.5 #2459

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Upgrade yarn to v3.8.5 #2459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
866c893
Upgrade yarn to v3.8.5
andreasgerstmayr Sep 26, 2024
2344b6b
ignore @types/react-router in depcheck
andreasgerstmayr Sep 26, 2024
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
3 changes: 3 additions & 0 deletions .gitattributes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# https://yarnpkg.com/getting-started/qa#which-files-should-be-gitignored
/.yarn/releases/** binary
/.yarn/plugins/** binary
2 changes: 1 addition & 1 deletion .github/workflows/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ FROM node:20.17.0
WORKDIR /workspace
COPY . .

RUN yarn install --frozen-lockfile && cd ./packages/jaeger-ui && yarn build
RUN yarn install --immutable && cd ./packages/jaeger-ui && yarn build
3 changes: 1 addition & 2 deletions .github/workflows/lint-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,10 @@ jobs:
with:
cache: yarn
node-version: '20'
- run: yarn install --frozen-lockfile
- run: yarn install --immutable
- name: Run depcheck
run: yarn run depcheck
- run: yarn lint
- run: yarn build
- name: Ensure PR is not on main branch
uses: jaegertracing/jaeger/.github/actions/block-pr-from-main-branch@main

2 changes: 1 addition & 1 deletion .github/workflows/release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
with:
cache: yarn
node-version: '20'
- run: yarn install --frozen-lockfile
- run: yarn install --immutable
- run: yarn lint
- run: yarn build
id: yarn-build
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/unit-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ jobs:
with:
cache: yarn
node-version: '20'
- run: yarn install --frozen-lockfile
- run: yarn install --immutable
- run: yarn coverage
- name: Upload coverage to codecov.io
uses: codecov/codecov-action@e28ff129e5465c2c0dcc6f003fc735cb6ae0c673 # v4.5.0
Expand Down
10 changes: 10 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,16 @@ yarn-error.log
.eslintcache
.vscode

# yarn
# https://yarnpkg.com/getting-started/qa#which-files-should-be-gitignored
.pnp.*
.yarn/*
!.yarn/patches
!.yarn/plugins
!.yarn/releases
!.yarn/sdks
!.yarn/versions
Comment on lines +31 to +33
Copy link
Member

Choose a reason for hiding this comment

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

Why would we not ignore these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied this list straight from the yarn docs here: https://yarnpkg.com/getting-started/qa#which-files-should-be-gitignored
There's an explanation in that FAQ entry for each path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regarding checking in yarn sources, afaics we have two options when using yarn v2+:

  • use the experimental corepack feature
  • or commit the yarn sources

see yarnpkg/berry#5748 for more details

Copy link
Member

Choose a reason for hiding this comment

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

corepack seems preferable, I am strongly against vendoring yarn code in the repo (feels insane to me that they would even propose this as a default setup).

In the issue there were mentions of problems with how corepack operates depending on versions. Probably not an issue for jaeger-ui.

Q: why yarn 3.8 and not 4.x?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason for yarn 3.8 was that we want to use cachi2 (used to ensure the build is hermetic, i.e. it caches dependencies to support building Jaeger UI without network access), which at the moment only supports yarn v3 (I assume v4 might also be supported, but I didn't test it).

Alternatively, we could also use npm. It also supports workspaces now (in case this was the reason to switch to yarn in the first place). What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I don't really follow. We have a yarn.lock file that ensures repeatable build. Are we worried about supply chain attack?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's about supply chain attacks. Our downstream builds need to be hermetic according to SLSA and include a Software Bill of Materials (SBOM) etc.

You're right, effectively the yarn package manager already supports these properties. We need to support multiple languages and package managers though, so this part of the build pipeline is handled by an external tool like cachi2 for our downstream builds.


# scripts
scripts/release-notes.py
scripts/draft-release.py
28 changes: 28 additions & 0 deletions .yarn/plugins/@yarnpkg/plugin-workspace-tools.cjs
View file
Open in desktop

Large diffs are not rendered by default.

875 changes: 875 additions & 0 deletions .yarn/releases/yarn-3.8.5.cjs
View file
Open in desktop

Large diffs are not rendered by default.

7 changes: 7 additions & 0 deletions .yarnrc.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
nodeLinker: node-modules

plugins:
- path: .yarn/plugins/@yarnpkg/plugin-workspace-tools.cjs
spec: "@yarnpkg/plugin-workspace-tools"

yarnPath: .yarn/releases/yarn-3.8.5.cjs
2 changes: 1 addition & 1 deletion BUILD.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ Holds GitHub Actions workflows used in CI and in release.

CodeCov is integrated into the unit tests workflow to report coverage data from `./packages/jaeger-ui`. When unit tests are added to Plexus, this integration will need to be updated to gather coverage data for Plexus as well.

[`yarn install --frozen-lockfile`](https://yarnpkg.com/lang/en/docs/cli/install/#toc-yarn-install-frozen-lockfile) ensures installs in CI fail if they would typically mutate the lockfile.
[`yarn install --immutable`](https://yarnpkg.com/cli/install#options) ensures installs in CI fail if they would typically mutate the lockfile.

## `tsconfig.json`

Expand Down
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ nvm use
Install dependencies via `yarn`:

```
yarn install --frozen-lockfile
yarn install --immutable
```

Make sure you have the Jaeger Query service running on http://localhost:16686. For example, you can run Jaeger all-in-one Docker image as described in the [documentation][aio-docs].
Expand Down
12 changes: 6 additions & 6 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
"typescript": "5.6.2"
},
"resolutions": {
"**/lodash": "^4.17.21",
"rollup": "^4.13.1"
},
"workspaces": {
Expand All @@ -41,17 +40,17 @@
]
},
"scripts": {
"build": "yarn workspaces run build",
"build": "yarn workspaces foreach --all run build",
"check-license": "./scripts/check-license.sh",
"coverage": "yarn workspaces run coverage",
"coverage": "yarn workspaces foreach --all run coverage",
"depcheck": "./scripts/run-depcheck.sh",
"eslint": "eslint --cache 'scripts/*.{js,jsx,ts,tsx}' 'packages/*/src/**/*.{js,jsx,ts,tsx}' 'packages/*/*.{js,jsx,ts,tsx,mts}'",
"fmt": "yarn prettier",
"lint": "npm-run-all -ln --parallel prettier-lint tsc-lint eslint check-license",
"prepare": "yarn workspace @jaegertracing/plexus prepublishOnly",
"postinstall": "yarn workspace @jaegertracing/plexus prepublishOnly",
"prettier": "prettier --write '{.,scripts}/*.{js,jsx,json,md,ts,tsx,mts}' 'packages/*/{src,demo/src}/**/!(layout.worker.bundled|react-vis).{css,js,jsx,json,md,ts,tsx}' 'packages/*/*.{css,js,jsx,json,md,ts,tsx,mts}'",
"prettier-lint": "prettier --list-different '{.,scripts}/*.{js,jsx,json,md,ts,tsx,mts}' 'packages/*/{src,demo/src}/**/!(layout.worker.bundled|react-vis).{css,js,jsx,json,md,ts,tsx}' 'packages/*/*.{css,js,jsx,json,md,ts,tsx,mts}' || (echo '*** PLEASE RUN yarn prettier AND RESUBMIT ***' && false)",
"test": "yarn workspaces run test",
"test": "yarn workspaces foreach --all run test",
"tsc-lint": "tsc --build",
"tsc-lint-debug": "tsc --listFiles",
"start": "cd packages/jaeger-ui && yarn start"
Expand All @@ -67,5 +66,6 @@
"hooks": {
"pre-commit": "npm-run-all -ln --parallel lint test"
}
}
},
"packageManager": "[email protected]"
}
7 changes: 4 additions & 3 deletions packages/jaeger-ui/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@
"@testing-library/react": "^15.0.7",
"@types/deep-freeze": "^0.1.1",
"@types/lodash": "^4.17.0",
"@types/object-hash": "^3.0.2",
"@types/react": "^18.3.3",
"@types/react-window": "^1.8.0",
"@types/redux-form": "^8.3.10",
"@types/react-helmet": "^6.1.5",
"@types/react-router": "^5.1.0",
"@types/react-router-dom": "^5.1.0",
"@types/react-window": "^1.8.0",
"@types/redux-actions": "2.2.1",
"@types/object-hash": "^3.0.2",
"@types/redux-form": "^8.3.10",
"@types/rollup-plugin-visualizer": "^4.2.1",
"@vitejs/plugin-legacy": "^5.4.1",
"@vitejs/plugin-react": "^4.3.1",
Expand Down
2 changes: 1 addition & 1 deletion packages/jaeger-ui/src/types/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export type ReduxState = {
error: ApiError | TNil;
};
embedded: EmbeddedState;
router: Router & {
router: typeof Router & {
location: Location;
};
services: {
Expand Down
10 changes: 5 additions & 5 deletions packages/plexus/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,18 @@
"d3-selection": "^3.0.0",
"d3-zoom": "^3.0.0",
"memoize-one": "6.0.0",
"viz.js": "1.8.1",
"react-icons": "^5.0.1"
"react-icons": "^5.0.1",
"viz.js": "1.8.1"
},
"scripts": {
"_tasks/build/lib/js": "node_modules/.bin/babel src --extensions '.tsx,.js' --out-dir lib",
"_tasks/build/lib/js": "babel src --extensions '.tsx,.js' --out-dir lib",
"_tasks/build/lib/types": "../../node_modules/.bin/tsc --build --force",
"_tasks/build/umd": "webpack --mode $NODE_ENV --config webpack.umd.config.js",
"_tasks/clean/dirs": "rimraf lib dist",
"_tasks/clean/worker": "rimraf src/LayoutManager/layout.worker*js*",
"_tasks/clean/worker": "rimraf 'src/LayoutManager/layout.worker*js*'",
"_tasks/bundle-worker": "webpack --mode $NODE_ENV --config webpack.layout-worker.config.js",
"_tasks/dev-server": "webpack-dev-server --mode $NODE_ENV --config webpack.dev.config.js",
"build": "NODE_ENV=production npm-run-all -ln --serial _tasks/clean/* _tasks/bundle-worker --parallel _tasks/build/**",
"build": "NODE_ENV=production npm-run-all -ln --serial '_tasks/clean/*' _tasks/bundle-worker --parallel '_tasks/build/**'",
"coverage": "echo 'NO TESTS YET'",
"prepublishOnly": "yarn build",
"start": "NODE_ENV='development' npm-run-all -ln --serial _tasks/clean/worker _tasks/bundle-worker --parallel '_tasks/bundle-worker --watch' _tasks/dev-server",
Expand Down
2 changes: 1 addition & 1 deletion scripts/generateDepcheckrcJaegerUI.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const packageNames = [
...babelConfiguration.plugins,
];

const otherPackages = ['jest-environment-jsdom'];
const otherPackages = ['jest-environment-jsdom', '@types/react-router'];

// Use the selected targetPackage for generating depcheckrcContent
const depcheckrcContent = {
Expand Down
Loading
Loading