Skip to content

Support sourcemap output for production builds#7087

Merged
aduth merged 6 commits intomainfrom
aduth-prod-webpack-devtool
Oct 7, 2022
Merged

Support sourcemap output for production builds#7087
aduth merged 6 commits intomainfrom
aduth-prod-webpack-devtool

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Oct 4, 2022

🛠 Summary of changes

  • Adds support to control the Webpack devtool option via environment variable, specifically to allow for explicit enabling of sourcemap output in a production builds (where otherwise sourcemaps would not be output by default).
  • Documents a debugging workflow for NewRelic JavaScript errors which relies on local sourcemap builds as of default sourcemap output in 4a29474 uses production sourcemaps

Notably, this would have helped with debugging #7070.

Alternatively, we could consider enabling sourcemaps in production builds, but debugging these errors does not yet appear to be a common enough need to impose an additional performance penalty on every production build. Edit: The pull request has been edited to use this approach as of 4a29474.

📜 Testing Plan

changelog: Internal, Build Tooling, Support sourcemap output for production builds to improve debugging
@aduth aduth requested a review from mitchellhenke October 4, 2022 20:46
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

const mode = isProductionEnv ? 'production' : 'development';
const hashSuffix = isProductionEnv ? '-[contenthash:8]' : '';
const devServerPort = process.env.WEBPACK_PORT;
const devtool = process.env.WEBPACK_DEVTOOL || (isProductionEnv ? false : 'eval-source-map');
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe one day we could load this from our configs in s3 or equivalent, so we could deploy these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe one day we could load this from our configs in s3 or equivalent, so we could deploy these?

Could you expand what you had in mind with S3? Technically we can enable these now to be deployed in production, I had simply opted for this route out of build performance's sake (and the difference may not be too much).

Copy link
Contributor

Choose a reason for hiding this comment

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

oh I just meant like, if we wanted to this to be deployed in staging or int but not prod, that's best configured through our s3 .yml files, but this doesn't read those

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've no strong opposition to deploying them everywhere (including production) if we think they'd be helpful. 🤷

Copy link
Contributor

Choose a reason for hiding this comment

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

🤷 I guess our application is open source anyways, and most of the time browsers don't download the sourcemaps right? Only when opening developer tools?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, any "risk" with the sourcemaps would be exposing the original source files if we wouldn't want to do that. And we already expose them via this repository 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I enabled sourcemap output by default in production in 4a29474 and the guidance in 688c564 + c0e9561. It required a bit of additional work since the sourcemaps seemed to use a different content hash than the original source file, so I switched from contenthash to chunkhash for Webpack output in ed8e011 (related docs).

@aduth aduth merged commit 128f0d6 into main Oct 7, 2022
@aduth aduth deleted the aduth-prod-webpack-devtool branch October 7, 2022 12:37
@jmdembe jmdembe mentioned this pull request Oct 11, 2022
jskinne3 pushed a commit that referenced this pull request Oct 12, 2022
* Support sourcemap output for production builds

changelog: Internal, Build Tooling, Support sourcemap output for production builds to improve debugging

* Set default production devtool to source-map

* Upgrade Webpack dependencies

* Use chunkhash for Webpack hash suffix

So that the sourcemap output files use the same hash as the JavaScript files

* Update guidance for production sourcemaps

* Fix words good
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.

2 participants