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

Node version update #878

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open

Conversation

ErvinRacz
Copy link

@ErvinRacz ErvinRacz commented Dec 9, 2024

Update node version

Summary

This PR updates the Node.js version used in GitHub workflows to Node.js 22 as Node.js 16 is now outdated. This required updates to several dependencies and tools to maintain compatibility and ensure smooth project functionality.

Changes

Node.js Upgrade:

  1. Updated GitHub workflow configuration to use Node.js 22.
    Re-generated package-lock.json with Node.js 22.

  2. Upgraded react-scripts to 5.0.1 to align with the new Node.js version.

  3. Cascading Updates:
    Addressed compatibility issues with certain MUI components, TypeScript, and Storybook:
    -- Upgraded MUI4 to MUI5
    -- Upgraded Storybook 6 to Storybook 7

  4. Adjusted testing config to ensure compatibility with the updated dependencies.

  5. Upgraded from React 17 to 18.

  6. Updated several other dependencies too (refer to the full list in the package.json).

With all these, detected vulnerabilities have been reduced from 177 vulnerabilities (6 low, 112 moderate, 50 high, 9 critical)
to 12 vulnerabilities (5 moderate, 7 high)

How to Use

The project should function exactly as it did previously with no noticeable changes in functionality.

Testing

Initial testing has been conducted, and the project builds successfully with Node.js 22.

  • Changelog entries added in the respective changelog/ directory (user-facing change, bug fix, security fix, update)
  • Inspected CI output for image differences: /boot and /usr size, packages, list files for any missing binaries, kernel modules, config files, kernel modules, etc.

re-run npm install with node 22 to check actual outdated deps
to update webpack and babel
updated react scripts to the latest version for compatibility with nodejs 22
increase node version in workflow and dockerfile
after new dep resolution, MUI got a minor version bumb
node version 22 & newer react-script version
Minor version bump of MUI deprecated the old prop name
executed prettier to format code
fixed ts version to 4.4.x due to compatibility issues until eslint
upgrade happens

See #issue_number
upgraded sb to 7 to use webpack 5
querystrings pkgs deprecated and replaced with native func
MUI 4 not compat with React 18 and some react scripts prefer react v18 with webpack 5
react-refresh didn't want to dedupe, and would cause dev runtime issues => override
With MUI 5, the theme and style engine became stricter, therefore comps
needed to be wrapped with the theme provider in some cases
MUI5 has a different styling engine, therefore the snaposhots have
changed
the default config for some components have changed, so standard variant
had to be specified
the style had to be brough back with the previous commits to the
original state, so snapshots have changed again
@tormath1
Copy link
Contributor

tormath1 commented Dec 19, 2024

CI starts to look good:

Jest: "global" coverage threshold for statements (34.95%) not met: 32.91%
Jest: "global" coverage threshold for branches (26.19%) not met: 24.47%
Jest: "global" coverage threshold for lines (35.26%) not met: 33.11%
Jest: "global" coverage threshold for functions (30.35%) not met: 29.68%

Test Suites: 15 passed, 15 total
Tests:       70 passed, 70 total
Snapshots:   51 passed, 51 total

we just need to:

  • increase the coverage
  • or/ reduce the threshold:
    "coverageThreshold": {
    "global": {
    "statements": 34.95,
    "branches": 26.19,
    "functions": 30.35,
    "lines": 35.26
    }

@ErvinRacz
Copy link
Author

  • increase the coverage
  • or/ reduce the threshold:

Great! I will go with the first option and add a couple of more tests since the threshold is almost met.

satisfy test coverage
@jepio jepio requested a review from Copilot December 19, 2024 12:43

Choose a reason for hiding this comment

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

Copilot reviewed 129 out of 144 changed files in this pull request and generated no comments.

Files not reviewed (15)
  • Dockerfile: Language not supported
  • frontend/package.json: Language not supported
  • frontend/public/index.html: Language not supported
  • frontend/src/tests/Common/snapshots/EmptyContent.spec.js.snap: Language not supported
  • frontend/src/tests/Common/snapshots/ListHeader.spec.js.snap: Language not supported
  • frontend/src/tests/Common/snapshots/ListSearch.spec.js.snap: Language not supported
  • frontend/src/tests/Common/snapshots/SectionHeader.spec.js.snap: Language not supported
  • .github/workflows/frontend.yml: Evaluated as low risk
  • frontend/src/components/Activity/ActivityContainer.stories.tsx: Evaluated as low risk
  • frontend/src/api/API.ts: Evaluated as low risk
  • frontend/src/tests/Common/SectionHeader.spec.js: Evaluated as low risk
  • frontend/src/tests/Common/PackagesList.spec.js: Evaluated as low risk
  • frontend/src/tests/Common/ListHeader.spec.js: Evaluated as low risk
  • frontend/src/tests/Common/ModalButton.spec.js: Evaluated as low risk
  • frontend/.storybook/preview.js: Evaluated as low risk
Comments suppressed due to low confidence (1)

frontend/src/TestHelpers/theme.ts:11

  • [nitpick] The change from green['500'] to green['800'] might affect the UI. Verify if this change was intended.
main: green['800'],
lodash required a newer TS version, but the eslint-config repo was archived and fixed on an older eslint version
@tormath1 tormath1 marked this pull request as ready for review December 20, 2024 11:47
@tormath1 tormath1 requested a review from a team December 20, 2024 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants