Skip to content

Commit

Permalink
fix: sourcemaps (#39301)
Browse files Browse the repository at this point in the history
## Description

- Returned the logic for generating the source maps. Source maps are
created for flags `REACT_APP_ENVIRONMENT=PRODUCTION` and
`REACT_APP_ENVIRONMENT=DEVELOPMENT` as before.
- Removed no longer used flag `DISABLE_ESLINT_PLUGIN`
- Removed no longer used `cra-bundle-analyzer` package and related
script.

EE PR — appsmithorg/appsmith-ee#6227

## Automation

/ok-to-test tags="@tag.All"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!CAUTION]
> 🔴 🔴 🔴 Some tests have failed.
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/13353994445>
> Commit: 88551e5
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=13353994445&attempt=3&selectiontype=test&testsstatus=failed&specsstatus=fail"
target="_blank">Cypress dashboard</a>.
> Tags: @tag.All
> Spec: 
> The following are new failures, please fix them before merging the PR:
<ol>
>
<li>cypress/e2e/Regression/ServerSide/Datasources/Snowflake_Basic_Spec.ts</ol>
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/identified-flaky-tests-65890b3c81d7400d08fa9ee3?branch=master"
target="_blank">List of identified flaky tests</a>.
> <hr>Sun, 16 Feb 2025 12:54:58 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [x] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Chores**
  - Updated the build process to consistently enforce quality checks.
- Enhanced debugging support by refining source mapping for both
production and development environments.
  - Removed an outdated analysis task to streamline the startup routine.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
KelvinOm authored Feb 16, 2025
1 parent 007f598 commit 19acf7c
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 139 deletions.
2 changes: 0 additions & 2 deletions app/client/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ fi
# build cra app
export REACT_APP_SENTRY_RELEASE=$GIT_SHA
export REACT_APP_CLIENT_LOG_LEVEL=ERROR
# Disable CRA built-in ESLint checks since we have our own config and a separate step for this
export DISABLE_ESLINT_PLUGIN=true
node --max-old-space-size=8192 scripts/build.js

echo "build finished"
48 changes: 28 additions & 20 deletions app/client/config/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,6 @@ module.exports = function (webpackEnv) {
const isEnvDevelopment = webpackEnv === "development";
const isEnvProduction = webpackEnv === "production";

// Set devtool based on environment
const devtool = isEnvProduction ? "source-map" : false;

// Variable used for enabling profiling in Production
// passed into alias object. Uses a flag if passed into the build command
const isEnvProductionProfile =
Expand Down Expand Up @@ -157,7 +154,9 @@ module.exports = function (webpackEnv) {
],
],
},
sourceMap: isEnvProduction ? true : isEnvDevelopment,
sourceMap:
process.env.REACT_APP_ENVIRONMENT === "PRODUCTION" ||
process.env.REACT_APP_ENVIRONMENT === "DEVELOPMENT",
},
},
].filter(Boolean);
Expand All @@ -166,14 +165,18 @@ module.exports = function (webpackEnv) {
{
loader: require.resolve("resolve-url-loader"),
options: {
sourceMap: isEnvProduction ? true : isEnvDevelopment,
sourceMap:
process.env.REACT_APP_ENVIRONMENT === "PRODUCTION" ||
process.env.REACT_APP_ENVIRONMENT === "DEVELOPMENT",
root: paths.appSrc,
},
},
{
loader: require.resolve(preProcessor),
options: {
sourceMap: true,
sourceMap:
process.env.REACT_APP_ENVIRONMENT === "PRODUCTION" ||
process.env.REACT_APP_ENVIRONMENT === "DEVELOPMENT",
},
},
);
Expand All @@ -188,7 +191,10 @@ module.exports = function (webpackEnv) {
mode: isEnvProduction ? "production" : "development",
// Stop compilation early in production
bail: isEnvProduction,
devtool: devtool,
devtool:
(process.env.REACT_APP_ENVIRONMENT === "PRODUCTION" && "source-map") ||
(process.env.REACT_APP_ENVIRONMENT === "DEVELOPMENT" &&
"cheap-module-source-map"),
// These are the "entry points" to our application.
// This means they will be the "root" imports that are included in JS bundle.
entry: paths.appIndexJs,
Expand Down Expand Up @@ -390,7 +396,7 @@ module.exports = function (webpackEnv) {
// Handle node_modules packages that contain sourcemaps
{
enforce: "pre",
exclude: /@babel(?:\/|\\{1,2})runtime/,
exclude: /(@babel[\\\/]runtime|node_modules)/,
test: /\.(js|mjs|jsx|ts|tsx|css)$/,
loader: require.resolve("source-map-loader"),
},
Expand Down Expand Up @@ -478,6 +484,7 @@ module.exports = function (webpackEnv) {
// directory for faster rebuilds.
cacheCompression: false,
compact: isEnvProduction,
exclude: /node_modules/,
},
},
// Process any JS outside of the app with Babel.
Expand All @@ -503,8 +510,12 @@ module.exports = function (webpackEnv) {
// Babel sourcemaps are needed for debugging into node_modules
// code. Without the options below, debuggers like VSCode
// show incorrect code and set breakpoints on the wrong lines.
sourceMaps: true,
inputSourceMap: true,
sourceMaps:
process.env.REACT_APP_ENVIRONMENT === "PRODUCTION" ||
process.env.REACT_APP_ENVIRONMENT === "DEVELOPMENT",
inputSourceMap:
process.env.REACT_APP_ENVIRONMENT === "PRODUCTION" ||
process.env.REACT_APP_ENVIRONMENT === "DEVELOPMENT",
},
},
// "postcss" loader applies autoprefixer to our CSS.
Expand All @@ -519,7 +530,9 @@ module.exports = function (webpackEnv) {
exclude: cssModuleRegex,
use: getStyleLoaders({
importLoaders: 1,
sourceMap: isEnvProduction ? true : isEnvDevelopment,
sourceMap:
process.env.REACT_APP_ENVIRONMENT === "PRODUCTION" ||
process.env.REACT_APP_ENVIRONMENT === "DEVELOPMENT",
modules: {
mode: "icss",
},
Expand All @@ -537,7 +550,9 @@ module.exports = function (webpackEnv) {
use: [
...getStyleLoaders({
importLoaders: 1,
sourceMap: isEnvProduction ? true : isEnvDevelopment,
sourceMap:
process.env.REACT_APP_ENVIRONMENT === "PRODUCTION" ||
process.env.REACT_APP_ENVIRONMENT === "DEVELOPMENT",
modules: {
mode: "local",
getLocalIdent: getCSSModuleLocalIdent,
Expand Down Expand Up @@ -586,13 +601,6 @@ module.exports = function (webpackEnv) {
].filter(Boolean),
},
ignoreWarnings: [
function ignoreSourcemapsloaderWarnings(warning) {
return (
(warning.module?.resource.includes("node_modules") &&
warning.details?.includes("source-map-loader")) ??
false
);
},
function ignorePackageWarnings(warning) {
return (
warning.module?.resource.includes(
Expand Down Expand Up @@ -625,7 +633,7 @@ module.exports = function (webpackEnv) {
threshold: 10240,
minRatio: 0.8,
}),
isEnvProduction &&
process.env.REACT_APP_ENVIRONMENT === "PRODUCTION" &&
new FaroSourceMapUploaderPlugin({
appId: process.env.REACT_APP_FARO_APP_ID,
appName: process.env.REACT_APP_FARO_APP_NAME,
Expand Down
4 changes: 1 addition & 3 deletions app/client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@
"!packages/**/build"
],
"scripts": {
"analyze": "yarn cra-bundle-analyzer",
"start": "DISABLE_ESLINT_PLUGIN=true BROWSER=none REACT_APP_ENVIRONMENT=DEVELOPMENT REACT_APP_CLIENT_LOG_LEVEL=debug node --max_old_space_size=4096 scripts/start.js",
"start": "BROWSER=none REACT_APP_ENVIRONMENT=DEVELOPMENT REACT_APP_CLIENT_LOG_LEVEL=debug node --max_old_space_size=4096 scripts/start.js",
"build": "./build.sh",
"build-airgap": "node download-assets.js && ./build.sh",
"test": "CYPRESS_BASE_URL=https://dev.appsmith.com cypress/test.sh",
Expand Down Expand Up @@ -308,7 +307,6 @@
"case-sensitive-paths-webpack-plugin": "^2.4.0",
"chalk": "^4.1.1",
"compression-webpack-plugin": "^10.0.0",
"cra-bundle-analyzer": "^0.1.0",
"css-loader": "^6.5.1",
"css-minimizer-webpack-plugin": "^3.2.0",
"cy-verify-downloads": "^0.0.5",
Expand Down
Loading

0 comments on commit 19acf7c

Please sign in to comment.