-
Notifications
You must be signed in to change notification settings - Fork 5
chore: remove CRA and move to a pure esbuild build #72
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
base: main
Are you sure you want to change the base?
Conversation
react-scripts is deprecated and its dependencies are a source of vulnerabilties. This moves the build to a pure esbuild more in line with other projects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To work it works as I could build the image and install it in docker desktop successfully.
I've left a few comments on things that I don't think we actually need but didn't verify all the packages changes one by one, which is where I trust your judgment. Let me know if you want me to go over the comments and address them or you wish to do it yourself :)
"esbuild": "^0.19.12", | ||
"esbuild-plugin-clean": "^1.0.1", | ||
"esbuild-plugin-svgr": "^1.0.0", | ||
"eslint": "^8.57.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like this eslint version is not compatible with the TS version we are using
"eslintConfig": { | ||
"extends": [ | ||
"react-app", | ||
"react-app/jest" | ||
] | ||
"airbnb", | ||
"airbnb/hooks", | ||
"plugin:@typescript-eslint/recommended", | ||
"plugin:jsx-a11y/recommended", | ||
"plugin:react/jsx-runtime", | ||
"prettier" | ||
], | ||
"parser": "@typescript-eslint/parser", | ||
"parserOptions": { | ||
"sourceType": "module", | ||
"ecmaVersion": "latest", | ||
"project": "./tsconfig.json" | ||
}, | ||
"plugins": [ | ||
"@typescript-eslint" | ||
], | ||
"settings": { | ||
"import/resolver": { | ||
"node": { | ||
"extensions": [ | ||
".js", | ||
".jsx", | ||
".ts", | ||
".tsx" | ||
] | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
II'm pretty sure most of these are not needed as we already specify in the .eslintrc.js
file
"@typescript-eslint/eslint-plugin": "^5.38.1", | ||
"eslint": "^8.24.0", | ||
"@esbuild-plugins/node-modules-polyfill": "^0.1.4", | ||
"@playwright/test": "^1.38.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to add this? Given there are no tests atm
Summary
react-scripts is deprecated and its dependencies are a source of vulnerabilities. This PR moves the build to a pure esbuild more in line with other projects. After this change, npm audit goes from 29 vulnerabilities to just 2.
public/ assets into build/
Tested:
Caveats / Follow-ups
Disclaimer: Codex agent helped heavily here