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

fix: packages upgrade and code refactoring #196

Merged
merged 14 commits into from
Nov 15, 2024
Merged

fix: packages upgrade and code refactoring #196

merged 14 commits into from
Nov 15, 2024

Conversation

melniiv
Copy link
Contributor

@melniiv melniiv commented Nov 13, 2024

Description

  1. Upgrade to Storybook 8
  2. Update to HDS 3.11
  3. Upgrade of all other packages to latest with exceptions:
  • sass-loader, css-loader, eslint and sass packages stay the same as they are breaking the sass webpack configs.
  • msw still version 1 as its not working with current setup still
  1. Refactoring:
  • fixed rollupjs build with upgraded packages
  • fixed storybook dev mode with upgraded packages
  • fixed most of the warnings related to sass and types
  • fixed tests
  • refactored all sass to be compatible with dart 3

Issues

  1. Still warnings in dev mode about the:
  • Deprecation The legacy JS API is deprecated and will be removed in Dart Sass 2.0.0.
    Not sure how to fix it in webpack config of the storybook main.js. The solution should be this configuration:
    sassLoaderOptions: { api: "modern", sassOptions: { includePaths: [path.join(__dirname, '..', 'src/common/styles')], }, },
    But as I have downgraded sass-loader, that might not work.
  • Reexporting of types from _generated. Also, I think our approach is correct, but warnings are not going away in dev mode.
  1. Tests
    For some reason with packages upgrade, the it.each not working anymore with resetMocks. When resetMocks commented out the tests are passing. somehow the jest spyOn breaks with mocks reset.

  2. docker compose does not work in podman. might be related to the user permissions.

  3. Niko got different results when running docker locally, for some reason storybook cli 7 was in use. has to be investigated.

Closes

DEV-XXX:

Related

Testing

Automated tests

Manual testing

Screenshots

Additional notes

@karisal-anders
Copy link
Contributor

karisal-anders commented Nov 13, 2024

Linter issues should be fixed (See below), and YARN_VERSION should be updated to at least 1.22.18 in Dockerfile as error @chromatic-com/[email protected]: The engine "yarn" is incompatible with this module. Expected version ">=1.22.18". Got "1.19.1" comes up otherwise when trying docker compose up --build. It seems docker doesn't work after that either, as then it complains about too old v20 node being used in docker:

error @typescript-eslint/[email protected]: The engine "node" is incompatible with this module. Expected version "^18.18.0 || ^20.9.0 || >=21.1.0". Got "20.5.1"

But we do have the ticket HCRC-119 for changing from helsinkitest to Red Hat UBI image, so that could help.

Linter issues in CI/CD pipeline:

Run yarn lint
yarn run v1.22.22
$ yarn eslint "src/**/*.{ts,tsx}" && yarn prettier ./src -c
$ /home/runner/work/react-helsinki-headless-cms/react-helsinki-headless-cms/node_modules/.bin/eslint 'src/**/*.{ts,tsx}'

/home/runner/work/react-helsinki-headless-cms/react-helsinki-headless-cms/src/common/utils/getIsValidHttpUrl.ts
Error:   6:12  error  '_' is defined but never used  @typescript-eslint/no-unused-vars

/home/runner/work/react-helsinki-headless-cms/react-helsinki-headless-cms/src/common/utils/makeLocaleStorageValue.ts
Error:   21:16  error  'e' is defined but never used  @typescript-eslint/no-unused-vars
Error:   [4](https://github.com/City-of-Helsinki/react-helsinki-headless-cms/actions/runs/11814274907/job/32913005512?pr=196#step:5:5)2:16  error  'e' is defined but never used  @typescript-eslint/no-unused-vars

✖ 3 problems (3 errors, 0 warnings)

@melniiv melniiv changed the title fix: packages upgrade fix: packages upgrade and code refactoring Nov 15, 2024
Copy link
Contributor

@nikomakela nikomakela left a comment

Choose a reason for hiding this comment

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

I got docker issues as well. With podman the container could not be created at all and with docker the storybook failed to start.

But since there are lots of fixes anyway for the actual app, I can approve this and the docker issues can be fixed in another PR.

@melniiv melniiv merged commit 4891272 into main Nov 15, 2024
1 check passed
@melniiv melniiv deleted the packages-upgrade3 branch November 15, 2024 09:24
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.

3 participants