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 to CRA 5 #4802

Merged
merged 3 commits into from
May 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 17 additions & 0 deletions dashboard/craco.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
const webpack = require("webpack");
const NodePolyfillPlugin = require('node-polyfill-webpack-plugin');

module.exports = {
webpack: {
configure: {
plugins: [
new NodePolyfillPlugin(), // add the required polyfills (not included in webpack 5)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is preventing us from doing something like:

        fallback: {
          process: require.resolve("process/browser"),
          stream: require.resolve("stream-browserify"),
          buffer: require.resolve("buffer"),
          timers: require.resolve("timers-browserify")
          ... // a line per each polyfill we (not really "we" but any random transitive dependency we use) need
        },

new webpack.ProvidePlugin({
process: 'process/browser.js',
Buffer: ['buffer', 'Buffer'],
}),
],
ignoreWarnings: [/Failed to parse source map/], // ignore source map warnings
},
},
};
14 changes: 8 additions & 6 deletions dashboard/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
"scripts": {
"build": "npm-run-all build-css compile-lang build-js",
"build-css": "sass src/ --no-source-map && yarn run copy-clr",
"build-js": "react-scripts build",
"build-js": "craco build",
"compile-lang": "formatjs compile-folder lang src/locales/ --ast --format simple",
"copy-clr": "shx cp ./node_modules/@clr/ui/clr-ui-dark.min.css public/clr-ui-dark.min.css && shx cp node_modules/@clr/ui/clr-ui.min.css public/clr-ui.min.css && shx cp ./node_modules/@clr/ui/clr-ui-dark.min.css.map public/clr-ui-dark.min.css.map && shx cp ./node_modules/@clr/ui/clr-ui.min.css.map public/clr-ui.min.css.map",
"eject": "react-scripts eject",
"eject": "craco eject",
"eslint-check": "eslint --config ./.eslintrc.json 'src/**/*.{js,ts,tsx}' --max-warnings=0",
"eslint": "eslint --config ./.eslintrc.json 'src/**/*.{js,ts,tsx}' --fix --max-warnings=0",
"extract-lang": "formatjs extract 'src/**/*.ts*' --out-file lang/en.json --id-interpolation-pattern '[sha512:contenthash:base64:6]' --format simple",
Expand All @@ -20,8 +20,8 @@
"prettier": "prettier --write 'src/**/*.{ts,tsx,scss}'",
"prettier-check": "prettier --check 'src/**/*.{ts,tsx,scss}'",
"start": "npm-run-all -p watch-css compile-lang start-js",
"start-js": "cross-env BROWSER=none react-scripts start",
"test": "yarn run build-css && yarn run compile-lang && react-scripts test",
"start-js": "cross-env BROWSER=none craco start",
"test": "yarn run build-css && yarn run compile-lang && craco test",
"ts-compile-check": "yarn run compile-lang && tsc -p tsconfig.json --noEmit",
"watch-css": "yarn run build-css && yarn run compile-lang && sass src/ --watch --no-source-map"
},
Expand Down Expand Up @@ -75,6 +75,7 @@
"yaml": "^2.1.1"
},
"devDependencies": {
"@craco/craco": "^6.4.3",
"@formatjs/cli": "^5.0.1",
"@improbable-eng/grpc-web-fake-transport": "^0.15.0",
"@testing-library/react": "^12.1.5",
Expand Down Expand Up @@ -104,11 +105,12 @@
"jest-plugin-context": "^2.9.0",
"mock-socket": "^9.1.4",
"moxios": "^0.4.0",
"node-polyfill-webpack-plugin": "^1.1.4",
"npm-run-all": "^4.1.5",
"postcss": "^8.4.14",
"postcss-scss": "^4.0.4",
"prettier": "^2.6.2",
"react-scripts": "^4.0.3",
"react-scripts": "^5.0.1",
"redux-mock-store": "^1.5.4",
"sass": "^1.52.1",
"shx": "^0.3.4",
Expand All @@ -122,7 +124,7 @@
"typescript": "^4.7.2"
},
"resolutions": {
"@babel/parser": "^7.15.3",
"@babel/parser": "^7.18.4",
Copy link
Contributor Author

@antgamdia antgamdia May 30, 2022

Choose a reason for hiding this comment

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

Quick reminder (as I also had to think twice): this resolutions object is there to prevent react 18 to be selected by default. Our app is not still prepared to run React 18.
Just upgrading to the latest minor version.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the explanation here - sounds like it could be worth a comment in the file at this point to our future selves?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, yarn/npm doesn't accept comments as part of the JSON file (even if jsonc exists...). See https://stackoverflow.com/questions/14221579/how-do-i-add-comments-to-package-json-for-npm-install to see some hacky ways to work around it.
Not sure if we should do something like that, but happy to explore some ways anyway in next PRs.

"@types/react": "^17.0.39"
},
"jest": {
Expand Down
Loading