Fix bundler generating invalid JavaScript for multi-line module.exports#3566
Fix bundler generating invalid JavaScript for multi-line module.exports#3566
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a JavaScript bundler bug where multi-line module.exports statements were not being properly removed, causing syntax errors in compiled workflow files. The fix enhances the removeExports function to track and remove multi-line export blocks by counting braces, and adds comprehensive tests to prevent regression.
- Enhanced bundler to handle multi-line
module.exports = { ... }statements - Added validation tests to ensure all bundled scripts produce valid JavaScript
- Improved documentation for
runs-onandconcurrencyfields with agentic workflow context - Regenerated workflow lock files with corrected bundled JavaScript (removing orphaned export lines)
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/bundler.go | Added multi-line module.exports tracking logic with brace counting to properly remove export blocks |
| pkg/workflow/bundler_test.go | Added comprehensive unit tests for multi-line export removal with various edge cases |
| pkg/workflow/bundler_validation_test.go | Added Node.js-based syntax validation tests for all bundled scripts |
| docs/src/content/docs/reference/frontmatter-full.md | Enhanced documentation for runs-on and concurrency fields with agentic workflow guidance |
| .github/workflows/*.lock.yml (14 files) | Regenerated workflow lock files removing orphaned updateActivationComment, and }; lines from bundled scripts |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if moduleExportsDepth == 0 { | ||
| inModuleExports = false | ||
| // Skip this closing line and continue | ||
| continue |
There was a problem hiding this comment.
The continue statement at line 205 is inside the inner for _, ch := range trimmed loop, so it will only skip to the next character iteration, not skip the rest of the line processing as intended by the comment "Skip this closing line and continue".
This should be a break statement to exit the inner character-processing loop, allowing the outer continue at line 210 to properly skip the entire line. The current code may process additional characters after finding the closing brace that ends the export block.
| continue | |
| break |
The JavaScript bundler was leaving orphaned code when removing multi-line
module.exportsstatements, causing syntax errors in bundled scripts likecreate_pull_request.cjs.Root Cause
The
removeExportsfunction only removed lines matching^\s*module\.exports\s*=, which stripped the opening line but left property names and closing braces:Changes
pkg/workflow/bundler.goremoveExportsto track multi-line export blocks using brace countingmodule.exports = { ... }blocks including all contentspkg/workflow/bundler_test.goTestRemoveExportsMultiLinecovering single/multi-property exports, nested objects, trailing commaspkg/workflow/bundler_validation_test.go(new)TestBundledScriptsHaveValidJavaScriptSyntaxto validate all 18 bundled scripts with Node.jsTestBundleCreatePullRequestScriptas specific regression testThe validation tests ensure all bundled scripts produce syntactically valid JavaScript, preventing runtime errors in GitHub Actions workflows.
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.