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

[web] Do not delete source map from production build #779

Merged
merged 2 commits into from
Sep 29, 2023

Conversation

dgdavid
Copy link
Contributor

@dgdavid dgdavid commented Sep 28, 2023

Problem

In the context of https://trello.com/c/hInThumV (internal link), we realized that source maps are missing in the production build.

Source map error: Error: request failed with status 404
Resource URL: http://127.0.0.90/cockpit/@localhost/agama/index.js
Source Map URL: index.js.map
Source map error: Error: request failed with status 404
Resource URL: http://127.0.0.90/cockpit/@localhost/agama/index.css
Source Map URL: index.css.map

They would have been useful for debugging the error we were facing. In fact, there is a Webpack recommendation about serving source maps in production too

We encourage you to have source maps enabled in production, as they are useful for debugging as well as running benchmark tests — https://webpack.js.org/guides/production/#source-mapping

The weird thing is that we already had the devtool webpack option set to "source-map", the value recommended for production builds. But, surprisingly, files were not there.

After closely checking the webpack configuration file, the culprit came to scene: the deleteOriginalAssets option of the CompressionWebpackPlugin. It was set as true and it does not honor the pattern given in test option.

Solution

Fortunately, it's something that was fixed long time ago. For us is just about switching from true to "keep-source-map" value.

Tests

Tested manually by checking the dist/ content after running the NODE_ENV=production npm run build command.

Click to show/hide ls -l dist/ output BEFORE the change
Sep 28 17:28 favicon.svg
Sep 28 17:28 fonts
Sep 28 17:28 index.css.gz
Sep 28 17:28 index.html.gz
Sep 28 17:28 index.js.gz
Sep 28 17:28 index.js.LICENSE.txt.gz
Sep 28 17:28 manifest.json
Sep 28 17:28 po.cs.js.gz
Sep 28 17:28 po.ja.js.gz
Sep 28 17:28 po.nl.js.gz
Sep 28 17:28 po.ru.js.gz
Sep 28 17:28 po.sv.js.gz
Sep 28 17:28 po.uk.js.gz
Click to show/hide ls -l dist/ output AFTER the change
Sep 28 17:31 favicon.svg
Sep 28 17:31 fonts
Sep 28 17:31 index.css.gz
Sep 28 17:31 index.css.map
Sep 28 17:31 index.html.gz
Sep 28 17:31 index.js.gz
Sep 28 17:31 index.js.LICENSE.txt.gz
Sep 28 17:31 index.js.map
Sep 28 17:31 manifest.json
Sep 28 17:31 po.cs.js.gz
Sep 28 17:31 po.ja.js.gz
Sep 28 17:31 po.nl.js.gz
Sep 28 17:31 po.ru.js.gz
Sep 28 17:31 po.sv.js.gz
Sep 28 17:31 po.uk.js.gz

By changing the `deleteOriginalAssets` CompressionWebpakPlugin option
from `true` to `"keep-source-map"`.

See https://webpack.js.org/plugins/compression-webpack-plugin/#deleteoriginalassets
@coveralls
Copy link

Coverage Status

coverage: 74.747%. remained the same when pulling 3264f51 on keep-source-map into f39b3b0 on master.

@imobachgs imobachgs merged commit c47701d into master Sep 29, 2023
9 checks passed
@imobachgs imobachgs deleted the keep-source-map branch September 29, 2023 11:35
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.

3 participants