[No-QA] fix: the web production build locally not working#71958
Conversation
|
|
686051a to
dfaf1a5
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariMacOS: Desktop |
| @@ -4,7 +4,7 @@ EXPENSIFY_URL=https://www.expensify.com/ | |||
| EXPENSIFY_PARTNER_NAME=chat-expensify-com | |||
| EXPENSIFY_PARTNER_PASSWORD=e21965746fd75f82bb66 | |||
| PUSHER_APP_KEY=268df511a204fbb60884 | |||
| USE_WEB_PROXY=false | |||
| USE_WEB_PROXY=true | |||
There was a problem hiding this comment.
| USE_WEB_PROXY=true | |
| USE_WEB_PROXY=false |
I don't think we need this change in the repo, otherwise on build prod it will set proxy thing which isn't needed.
There was a problem hiding this comment.
Yep, let's remove this change and add info to readme
| @@ -29,7 +29,7 @@ | |||
| "web": "./scripts/set-pusher-suffix.sh && concurrently npm:web-proxy npm:web-server", | |||
| "web-proxy": "ts-node web/proxy.ts", | |||
| "web-server": "./scripts/start-dev-with-auto-restart.sh", | |||
| "build": "webpack --config config/webpack/webpack.common.ts --env file=.env.production && ts-node ./scripts/combine-web-sourcemaps.ts", | |||
| "build": "NODE_OPTIONS=--max_old_space_size=8192 webpack --config config/webpack/webpack.common.ts --env file=.env.production && ts-node ./scripts/combine-web-sourcemaps.ts", | |||
There was a problem hiding this comment.
Do we really need this change?
There was a problem hiding this comment.
For me it worked without it
| @@ -75,7 +75,7 @@ | |||
| "react-compiler-compliance-check": "ts-node scripts/react-compiler-compliance-check.ts", | |||
| "generate-search-parser": "peggy --format es -o src/libs/SearchParser/searchParser.js src/libs/SearchParser/searchParser.peggy src/libs/SearchParser/baseRules.peggy", | |||
| "generate-autocomplete-parser": "peggy --format es -o src/libs/SearchParser/autocompleteParser.js src/libs/SearchParser/autocompleteParser.peggy src/libs/SearchParser/baseRules.peggy && ./scripts/parser-workletization.sh src/libs/SearchParser/autocompleteParser.js", | |||
| "web:prod": "http-server ./dist --cors", | |||
| "web:prod": "concurrently \"npm:web-proxy\" \"http-server ./dist --cors --port 8080 -P http://localhost:9000\"", | |||
There was a problem hiding this comment.
| "web:prod": "concurrently \"npm:web-proxy\" \"http-server ./dist --cors --port 8080 -P http://localhost:9000\"", | |
| "web:dist": "concurrently \"npm:web-proxy\" \"http-server ./dist --cors --port 8080 -P http://localhost:9000\"", |
As this will be a common for staging/prod, how about any common name?
|
Also Tagging @blazejkustra and @LukasMod for review 👀 |
| @@ -4,7 +4,7 @@ EXPENSIFY_URL=https://www.expensify.com/ | |||
| EXPENSIFY_PARTNER_NAME=chat-expensify-com | |||
| EXPENSIFY_PARTNER_PASSWORD=e21965746fd75f82bb66 | |||
| PUSHER_APP_KEY=268df511a204fbb60884 | |||
| USE_WEB_PROXY=false | |||
| USE_WEB_PROXY=true | |||
There was a problem hiding this comment.
Yep, let's remove this change and add info to readme
There was a problem hiding this comment.
This feels a bit generated to me, sorry 😆
There was a problem hiding this comment.
Btw I don't think we need this much information, the part in SETUP_WEB should be enough
There was a problem hiding this comment.
This feels a bit generated to me, sorry 😆
Hehe No problem. I wrote the some and refactored with generation. I thought I might be helpful
| @@ -29,7 +29,7 @@ | |||
| "web": "./scripts/set-pusher-suffix.sh && concurrently npm:web-proxy npm:web-server", | |||
| "web-proxy": "ts-node web/proxy.ts", | |||
| "web-server": "./scripts/start-dev-with-auto-restart.sh", | |||
| "build": "webpack --config config/webpack/webpack.common.ts --env file=.env.production && ts-node ./scripts/combine-web-sourcemaps.ts", | |||
| "build": "NODE_OPTIONS=--max_old_space_size=8192 webpack --config config/webpack/webpack.common.ts --env file=.env.production && ts-node ./scripts/combine-web-sourcemaps.ts", | |||
There was a problem hiding this comment.
For me it worked without it
| @@ -75,7 +75,7 @@ | |||
| "react-compiler-compliance-check": "ts-node scripts/react-compiler-compliance-check.ts", | |||
| "generate-search-parser": "peggy --format es -o src/libs/SearchParser/searchParser.js src/libs/SearchParser/searchParser.peggy src/libs/SearchParser/baseRules.peggy", | |||
| "generate-autocomplete-parser": "peggy --format es -o src/libs/SearchParser/autocompleteParser.js src/libs/SearchParser/autocompleteParser.peggy src/libs/SearchParser/baseRules.peggy && ./scripts/parser-workletization.sh src/libs/SearchParser/autocompleteParser.js", | |||
| "web:prod": "http-server ./dist --cors", | |||
| "web:prod": "concurrently \"npm:web-proxy\" \"http-server ./dist --cors --port 8080 -P http://localhost:9000\"", | |||
| npm run build | ||
|
|
||
| # 2. Run the production server | ||
| npm run web:prod |
There was a problem hiding this comment.
Here as well
| npm run web:prod | |
| npm run web:dist |
faebb72 to
74ba741
Compare
|
|
||
| The `web:dist` command starts both the proxy server (port 9000) and web server (port 8080) concurrently. Access the application at **http://localhost:8080** | ||
|
|
||
| **Important**: You must set `USE_WEB_PROXY=true` before running `web:dist` to avoid CORS errors when testing locally. |
There was a problem hiding this comment.
Are you sure it’s not supposed to be the first step? The solution works for me if I set the flag before running npm run build, since it uses that environment variable during the build process. When I did it in described order I got cors errors.
There was a problem hiding this comment.
Yea thanks @LukasMod I think I was running cached build. Forgot during that. Fixed it.
| * [React StrictMode](contributingGuides/STRICT_MODE.md) | ||
| * [Left Hand Navigation(LHN)](contributingGuides/LEFT_HAND_NAVIGATION.md) | ||
| * [HybridApp - additional info & troubleshooting](contributingGuides/HYBRID_APP.md) | ||
| * [Running Production Builds Locally](contributingGuides/PRODUCTION_BUILD.md) |
There was a problem hiding this comment.
This can be removed as well 👍
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/thienlnam in version: 9.2.30-0 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.2.30-6 🚀
|
Explanation of Change
Problem: The production build script was misconfigured, causing developers to encounter CORS errors, 404 responses, and "You appear to be offline" messages when attempting to run production builds locally.
Root Cause:
Fixed Issues
$ #69854
PROPOSAL: #69854 (comment)
Tests
Prerequisites:
Test 1: Production Build - Basic Functionality
- Proxy server: Proxy server listening at http://localhost:9000
- Web server: Available on: http://127.0.0.1:8080
Expected Result: App loads and shows as online
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop