Skip to content

fix: reject minification of implicit simple output - #2182

Open
ryanchou1994 wants to merge 1 commit into
handlebars-lang:masterfrom
ryanchou1994:fix/implicit-simple-minification
Open

ryanchou1994 wants to merge 1 commit into
handlebars-lang:masterfrom
ryanchou1994:fix/implicit-simple-minification

Conversation

@ryanchou1994

Copy link
Copy Markdown

Running echo test | handlebars -m -i- currently exits successfully and prints undefined. An unnamed template implicitly enables simple output, but that happens after the existing simple-output/minification check.

Apply the same check to a single unnamed template before compiling. This reports Unable to minimize simple output with exit status 1 and no stdout, matching explicit --simple --min behavior. Named inline and stdin templates still produce executable minimized output. Existing error precedence is preserved.

Adds real CLI regressions for both input forms, named-minification rendering controls, and a guard-precedence test. The two new rejection tests fail on the original code.

Validation: Linux build, lint and default test suite pass (655 passed, 27 skipped; coverage thresholds pass). The Mac suite also passes with command-scoped init.defaultBranch=master to match existing Git test fixtures; lint passes. Separate integration/browser/Windows matrices were not run locally.

Refs #1525. This does not add minification support for unnamed simple output.

Before creating a pull-request, please check https://github.com/handlebars-lang/handlebars.js/blob/master/CONTRIBUTING.md first.

Generally we like to see pull requests that

  • Please don't start pull requests for security issues. Instead, file a report at https://www.npmjs.com/advisories/report?package=handlebars
  • Maintain the existing code style
  • Are focused on a single change (i.e. avoid large refactoring or style adjustments in untouched code if not the primary goal of the pull request)
  • Have good commit messages
  • Have tests
  • Have the typings (types/index.d.ts) updated on every API change. N/A: no API or type signature changes.
  • Don't significantly decrease the current code coverage (see coverage/lcov-report/index.html)
  • Please target the master branch in the PR.

Copilot AI lite review requested due to automatic review settings September 12, 2026 08:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@jaylinski
jaylinski requested a lite review from Copilot September 13, 2026 08:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@ryanchou1994

Copy link
Copy Markdown
Author

Hi @jaylinski, friendly ping: the workflows on this PR are still waiting for approval to run. Happy to adjust anything if needed. Thanks!

This branch has not been deployed

No deployments
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