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

Update React Scripts, ESLint and testing packages #3659

Merged
merged 10 commits into from
Dec 18, 2023

Conversation

tacigar
Copy link
Member

@tacigar tacigar commented Dec 16, 2023

  • Update react-scripts version to the latest.
  • Update and clean up ESLint packages
  • Update and clean up testing packages (update testing-library and remove enzyme)

This PR includes code changes, but they are all fixes to pass the linter.

Comment on lines -52 to -58
"eslint-config-prettier": "^6.10.0",
"eslint-plugin-import": "^2.16.0",
"eslint-plugin-jest": "^23.8.2",
"eslint-plugin-jsx-a11y": "^6.2.1",
"eslint-plugin-prettier": "^3.1.2",
"eslint-plugin-react": "^7.12.4",
"eslint-plugin-react-hooks": "^2.5.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

These ESLint plugins are included in react-scripts eslint config.

@tacigar tacigar changed the title [wip] Update React Scripts and ESLint packages [wip] Update React Scripts, ESLint and testing packages Dec 17, 2023

import ServiceBadge from './ServiceBadge';
import { render, screen } from '@testing-library/react';
Copy link
Member Author

@tacigar tacigar Dec 17, 2023

Choose a reason for hiding this comment

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

Use @testing-library/react instead of enzyme since all other test code already uses it.
Now we can remove enzyme from package.json.

"@types/enzyme": "^3.10.5",
"@types/enzyme-adapter-react-16": "^1.0.6",
"@testing-library/jest-dom": "6.1.5",
"@testing-library/react": "12.1.5",
Copy link
Member Author

Choose a reason for hiding this comment

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

This is not the latest version.
testing-library/react 13+ requires React v18, but we still use React v16, so I could not update to the latest version (14.1.2).

https://www.npmjs.com/package/@testing-library/react

'prettier/@typescript-eslint',
'prettier/react',
],
extends: ['plugin:prettier/recommended', 'react-app', 'react-app/jest'],
Copy link
Member Author

Choose a reason for hiding this comment

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

Remove

  • airbnb-typescript
  • prettier/@typescript-eslint
  • prettier/react

and extend react-scrtips eslint configs.

Comment on lines -49 to -50
"enzyme": "^3.7.0",
"enzyme-adapter-react-16": "^1.7.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

Now enzyme is not used. All test code is only using testing-library.

"@testing-library/jest-dom": "^5.5.0",
"@testing-library/react": "^10.0.2",
"@testing-library/react-hooks": "^3.2.1",
"@testing-library/user-event": "^10.0.2",
Copy link
Member Author

Choose a reason for hiding this comment

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

Currently @testing-library/user-event is not used in the test code.

"fetch-mock": "^9.4.0",
"history": "^4.10.1",
"http-proxy-middleware": "2.0.6",
"jest-environment-jsdom-sixteen": "^1.0.3",
Copy link
Member Author

Choose a reason for hiding this comment

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

https://www.npmjs.com/package/jest-environment-jsdom-sixteen

jest@26 ships with jsdom@16, so there is no reason to use this module

const { getByTestId } = render(<LanguageSelector />);
const changeLanguageButton = getByTestId('change-language-button');
render(<LanguageSelector />);
const changeLanguageButton = screen.getByTestId('change-language-button');
Copy link
Member Author

Choose a reason for hiding this comment

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

const components = await waitForElement(() =>
getAllByTestId('dependencies-graph'),
);
const components = await screen.findAllByTestId('dependencies-graph');
Copy link
Member Author

Choose a reason for hiding this comment

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

@tacigar tacigar changed the title [wip] Update React Scripts, ESLint and testing packages Update React Scripts, ESLint and testing packages Dec 18, 2023
@tacigar tacigar marked this pull request as ready for review December 18, 2023 00:33
@tacigar
Copy link
Member Author

tacigar commented Dec 18, 2023

Thanks @anuraaga !!

@tacigar tacigar merged commit 98e4c03 into openzipkin:master Dec 18, 2023
12 checks passed
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