Skip to content

Conversation

@agibson-godaddy
Copy link
Contributor

@agibson-godaddy agibson-godaddy commented Aug 22, 2025

Issue: https://godaddy-corp.atlassian.net/browse/MWC-18446

Summary

Our intention has always been to not ship sourcemap files in production code, and as part of that we attempt to strip the sourcemap comments from minified files.

At some point the comment style must have changed and it was no longer getting stripped. I think they went from /* style comments to // style in JS, and that accounts for the change.

Rather than accounting for the new comment format I opted to implement the TODO method of not generating them at all if we're in the build or deploy tasks.

QA

  1. Use this PR as a development version of Sake
  2. Work in a repo that contains both SASS and JS files. I'm using https://github.com/gdcorp-partners/woocommerce-customer-order-csv-export
  3. Run sake compile
  4. Inspect a compiled CSS file
    • It contains a sourcemap comment at the end like this: /*# sourceMappingURL=wc-customer-order-csv-export-admin.min.css.map */
  5. Inspect a minified JS file
    • It contains a sourcemap comment at the end like this: //# sourceMappingURL=wc-customer-order-csv-export-admin.min.js.map
  6. Run sake build
  7. Inspect the same compiled CSS file but inside the build/ directory
    • It does NOT contain a sourcemap comment
  8. Inspect the same compiled JS file but inside the build/ directory
    • It does NOT contain a sourcemap comment

@agibson-godaddy agibson-godaddy marked this pull request as ready for review August 22, 2025 12:51
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses an issue where source maps were still being included in production builds despite attempts to strip them. Instead of fixing the regex pattern that strips sourcemap comments, the implementation now prevents sourcemap generation entirely during build and deploy tasks.

  • Removes the regex-based sourcemap comment stripping from the copy task
  • Conditionally generates sourcemaps only when not running build/deploy tasks
  • Adds a new isBuildTask() helper method to detect build-related CLI invocations

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
tasks/copy.js Removes the regex pattern that was attempting to strip sourcemap comments
tasks/compile.js Conditionally wraps sourcemap generation with isBuildTask() checks for all asset types
pipes/scripts.js Updates the JavaScript compilation pipeline to conditionally generate sourcemaps
lib/sake.js Adds the isBuildTask() helper method to detect build-related CLI arguments

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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