Skip to content

Fix build verification by stripping debug calls from all builds #1753

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

Merged
merged 5 commits into from
Jun 2, 2025

Conversation

wycats
Copy link
Contributor

@wycats wycats commented May 30, 2025

Summary

This PR fixes the CI build verification failure by integrating the existing @glimmer/local-debug-babel-plugin into the build pipeline to automatically strip debug code from all published builds.

Problem

Debug functions like check(), expect(), and unwrap() from @glimmer/debug were appearing in production builds. These functions are meant only for Glimmer VM developers working on the codebase itself and should never ship to users.

Solution

Instead of relying on a verification script that checks for debug code after the fact, we now automatically strip it during the build process using the Babel plugin that was already in the codebase but not integrated.

Changes

  • Build Configuration: Added the Babel plugin to both ESM and CommonJS build pipelines
  • Dependencies: Added @rollup/plugin-babel to enable Babel transformations
  • CI Cleanup: Removed the build verification script and its CI reference (no longer needed)
  • Documentation: Added comprehensive guides explaining build constraints and debug assertions

Impact

  • ✅ Developers can continue using debug assertions freely (check(), expect(), etc.)
  • ✅ Zero runtime cost - all debug code is completely removed from published packages
  • ✅ Automatic enforcement - no manual verification needed
  • ✅ Better documentation for future contributors

Technical Details

The Babel plugin transforms debug code during build:

  • check(value, checker)value
  • expect(...) → removed entirely
  • unwrap(value)value
  • recordStackSize() → removed entirely
  • Type validators like CheckInterface() => true

This happens for ALL builds (both development and production) because these debug functions are only for Glimmer VM development, not for users of Glimmer.

Copy link
Contributor

github-actions bot commented May 30, 2025

This PRmain
Dev
588K └─┬ .
169K   ├── runtime
160K   ├── syntax
100K   ├── compiler
 58K   ├── opcode-compiler
 27K   ├── manager
 24K   ├── validator
 11K   ├── program
8.9K   ├── reference
7.2K   ├── destroyable
6.3K   ├── util
4.3K   ├── node
3.4K   ├── global-context
2.5K   ├── wire-format
1.0K   ├── vm
969B   ├── encoder
844B   ├── vm-babel-plugins
606B   └── owner
588K └─┬ .
169K   ├── runtime
160K   ├── syntax
100K   ├── compiler
 58K   ├── opcode-compiler
 27K   ├── manager
 24K   ├── validator
 11K   ├── program
8.9K   ├── reference
7.2K   ├── destroyable
6.3K   ├── util
4.3K   ├── node
3.4K   ├── global-context
2.5K   ├── wire-format
1.0K   ├── vm
969B   ├── encoder
844B   ├── vm-babel-plugins
606B   └── owner
Prod
231K └─┬ .
 70K   ├── syntax
 63K   ├── runtime
 48K   ├── compiler
 18K   ├── opcode-compiler
7.9K   ├── manager
5.1K   ├── validator
4.8K   ├── program
3.6K   ├── reference
2.4K   ├── util
2.1K   ├── node
1.6K   ├── wire-format
1.5K   ├── destroyable
737B   ├── vm
594B   ├── global-context
516B   ├── encoder
469B   ├── vm-babel-plugins
155B   └── owner
231K └─┬ .
 70K   ├── syntax
 63K   ├── runtime
 48K   ├── compiler
 18K   ├── opcode-compiler
7.9K   ├── manager
5.1K   ├── validator
4.8K   ├── program
3.6K   ├── reference
2.4K   ├── util
2.1K   ├── node
1.6K   ├── wire-format
1.5K   ├── destroyable
737B   ├── vm
594B   ├── global-context
516B   ├── encoder
469B   ├── vm-babel-plugins
155B   └── owner

@wycats wycats force-pushed the fix/strip-debug-calls-from-builds branch from 9a226e2 to ddaa9ff Compare May 30, 2025 18:17
wycats added 5 commits May 31, 2025 00:27
Debug functions like check(), expect(), and unwrap() from @glimmer/debug
are meant only for local development and must not appear in any published
builds. This commit adds a Babel plugin to the build pipeline that strips
these calls from both development and production builds.

Changes:
- Add @glimmer/local-debug-babel-plugin to build pipeline
- Configure plugin to strip debug calls after TypeScript compilation
- Remove build-verify.mjs and its CI reference as debug stripping is now automatic
- Work around pnpm ecosystem peer dependency conflicts in CI floating dependencies test
- Add comprehensive documentation about build constraints
- Document debug assertion usage for developers

The solution uses the existing @glimmer/local-debug-babel-plugin that
was already in the codebase but not integrated into the build process.
This ensures developers can continue using debug assertions freely while
the build system automatically removes them from published packages.
The build verification script was deleted as part of integrating automatic
debug stripping via Babel plugin. The repo metadata needs to be updated
to reflect this change.
Improve robustness of the metadata update script by adding:
- Try-catch blocks around pnpm execution and JSON parsing
- Validation that package.json files exist before reading
- Proper error messages for debugging
- Type assertions to fix TypeScript safety issues
- Replace process.exit() with thrown errors per linting rules

Also removed problematic @pnpm/* dependencies from repo-metadata
package.json and fixed the floating dependencies CI job.
Remove npm metadata artifacts that were contaminating package.json during
development (bugs, readme errors, _id, homepage, author format changes).

Exclude root package.json from package updater processing to prevent
property reordering that causes Verify CI check failures.
@wycats wycats force-pushed the fix/strip-debug-calls-from-builds branch from a8e6b19 to cea7cf0 Compare May 31, 2025 08:20
@NullVoxPopuli NullVoxPopuli merged commit 52bb539 into main Jun 2, 2025
9 checks passed
@NullVoxPopuli NullVoxPopuli deleted the fix/strip-debug-calls-from-builds branch June 2, 2025 21:19
@github-actions github-actions bot mentioned this pull request Jun 2, 2025
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