Skip to content

[BUGFIX] Invoke argument-less helpers in attribute position in strict mode - #21516

Draft
NullVoxPopuli wants to merge 1 commit into
mainfrom
nvp/fix-strict-attr-helper-invoke
Draft

[BUGFIX] Invoke argument-less helpers in attribute position in strict mode#21516
NullVoxPopuli wants to merge 1 commit into
mainfrom
nvp/fix-strict-attr-helper-invoke

Conversation

@NullVoxPopuli

@NullVoxPopuli NullVoxPopuli commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Fixes #21040

Supersedes #21508

  • see here for history and context

Blocked on:

Context:

@NullVoxPopuli NullVoxPopuli changed the title Invoke argument-less helpers in attribute position in strict mode [BUGFIX] Invoke argument-less helpers in attribute position in strict mode Jul 23, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📊 Size report

Tarball size1.2 MB1.2 MB

dist/dev   0.06%↑

File Before (Size / Brotli) After (Size / Brotli)
./packages/shared-chunks/index-{hash}.js 62.6 kB / 12.5 kB 2%↑63.8 kB / 2%↑12.8 kB
Total (Includes all files) 2.1 MB / 498 kB 0.06%↑2.1 MB / 0.06%↑498.3 kB

dist/prod   0.06%↑

File Before (Size / Brotli) After (Size / Brotli)
./packages/shared-chunks/index-{hash}.js 59.8 kB / 12.1 kB 2%↑61 kB / 2%↑12.4 kB
Total (Includes all files) 1.9 MB / 455.4 kB 0.06%↑1.9 MB / 0.06%↑455.7 kB

smoke-tests/v2-app-template/dist   0.05%↑

File Before (Size / Brotli) After (Size / Brotli)
./assets/api-{hash}.js 287.9 kB / 77.5 kB 0.06%↑288.1 kB / 0.06%↑77.5 kB
Total (Includes all files) 351.2 kB / 97.8 kB 0.05%↑351.4 kB / 0.03%↑97.8 kB

smoke-tests/v2-app-hello-world-template/dist   0.1%↑

File Before (Size / Brotli) After (Size / Brotli)
./assets/main-{hash}.js 132.2 kB / 36.9 kB 0.1%↑132.4 kB / 0.1%↑36.9 kB
Total (Includes all files) 132.6 kB / 37 kB 0.1%↑132.7 kB / 0.2%↑37.1 kB

🤖 This report was automatically generated by wyvox/pkg-size

Copilot AI left a comment

Copy link
Copy Markdown

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 fixes a strict-mode inconsistency where argument-less helpers (and plain functions with helper managers) used in attribute value / attribute interpolation were not implicitly invoked, causing stringification (e.g. Function.prototype.toString) instead of producing the helper’s return value.

Changes:

  • Introduces DynamicAttrValue() to compile strict-mode attribute-position expressions, invoking bare lexical helpers/functions (and rejecting components) while preserving value semantics otherwise.
  • Routes dynamic element attributes, component invocation attributes (...attributes passthrough), and Concat parts through DynamicAttrValue() instead of the generic expr() path.
  • Adds a new strict-mode integration test suite covering helper/function invocation in attribute position (including interpolation, trusting attrs, updates, and error cases).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
packages/@glimmer/opcode-compiler/lib/syntax/statements.ts Uses DynamicAttrValue() for dynamic attribute compilation paths (plain + trusting, element + component attrs).
packages/@glimmer/opcode-compiler/lib/syntax/expressions.ts Compiles Concat parts via DynamicAttrValue() to ensure attribute interpolations invoke bare lexical helpers/functions correctly.
packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/vm.ts Adds DynamicAttrValue() helper implementing compile-time OptionalComponentOrHelper resolution for bare lexical symbols in attribute-position.
packages/@glimmer-workspace/integration-tests/test/strict-mode-test.ts Adds strict mode: attribute position test suite validating the intended behavior and guarding regressions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@NullVoxPopuli

Copy link
Copy Markdown
Contributor Author

That's a summary, not a review. Silly copilot

@kategengler

Copy link
Copy Markdown
Member

Making it a draft since it needs an RFC

@kategengler
kategengler marked this pull request as draft August 11, 2026 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Strict mode: helpers inside attribute concats are not auto-invoked

5 participants