Skip to content

feat(core): scope getVariables() per sub-comp instance (PR 2/4)#601

Merged
jrusso1020 merged 2 commits intomainfrom
feat/get-variables-subcomp
May 4, 2026
Merged

feat(core): scope getVariables() per sub-comp instance (PR 2/4)#601
jrusso1020 merged 2 commits intomainfrom
feat/get-variables-subcomp

Conversation

@jrusso1020
Copy link
Copy Markdown
Collaborator

What

Building on PR #600 (getVariables() helper + --variables flag), this PR scopes the helper so embedded sub-compositions see their own per-instance values. The same composition source can now be embedded N times with different content via data-variable-values on each host.

This is PR 2 of a 4-PR stack — based on feat/get-variables. Will retarget to main after PR #600 merges.

Why

data-variable-values was already documented as the per-instance attribute for nested compositions, but readers had to hand-roll JSON.parse(host.dataset.variableValues) and there was no scoping system — every sub-comp script had to re-implement the same pattern. This PR routes the values to a scoped getVariables() so authors use one API in both top-level and sub-comp contexts.

How

[host element]                                    [sub-comp source HTML]
data-variable-values='{...}'        +    <html data-composition-variables='[...]'>
                                                  ^ declared defaults
        ↓                                                  ↓
        └──────────── compositionLoader ────────────────────┘
                            ↓
            window.__hfVariablesByComp[compId] = merged
                            ↓
                  scripts wrapped by compositionScoping
                            ↓
            __hyperframes.getVariables() → scoped lookup

Three small surgical changes:

  • compositionLoader.ts — before injecting wrapped scripts, parse the host's data-variable-values JSON, merge it over readDeclaredDefaults(doc.documentElement) (reused from PR 1's runtime helper), and write the result to window.__hfVariablesByComp[compositionId]. Skipped when both sides are empty so the table only grows when there's actual data. Inline templates (no separate <html> root) get host overrides only.

  • compositionScoping.ts — wrapper IIFE now takes a fourth parameter __hyperframes alongside the existing scoped document / gsap / window. Same shadowing pattern that already works for the other three. The scoped __hyperframes overrides getVariables to read from __hfVariablesByComp[__hfCompId] and returns a fresh object each call so script mutations don't leak into the shared table.

  • getVariables.tsreadDeclaredDefaults becomes a named export (was a private helper) so the loader reuses the exact same defaults-extraction logic the top-level helper uses.

Top-level scripts that aren't wrapped by compositionScoping keep calling window.__hyperframes.getVariables() and get the unscoped path from PR 1 (declared defaults + CLI --variables override).

Test plan

  • Unit tests added/updated
    • 3 new compositionScoping.test.ts — scoped getVariables invocation routes to __hfVariablesByComp[compId]; missing-entry returns {}; mutation of the returned object doesn't leak into the shared table.
    • 5 new compositionLoader.test.ts — merge order (host overrides win); declared-only path when host has no data-variable-values; empty-skip when neither side has data; invalid-host-JSON falls through to declared defaults; per-instance scoping across two hosts that share a source.
    • 3 new getVariables.test.ts — covering the newly-public readDeclaredDefaults export (null root, valid attribute, invalid JSON / non-array).
    • All 622 existing core tests green.
  • Manual flow walkthrough — host with data-variable-values='{"title":"Pro"}' → sub-comp's __hyperframes.getVariables() returns {title:"Pro", ...declaredDefaults}. Two hosts pointing at the same source with different overrides → each script sees its own values.
  • Documentation updated
    • docs/concepts/compositions.mdx — switched the sub-comp example from JSON.parse(host.dataset.variableValues) to __hyperframes.getVariables(); added declared-defaults pattern and per-instance scoping note.
    • docs/concepts/data-attributes.mdx — clarified per-instance behavior on data-variable-values.

Backwards compatibility

Fully backwards compatible. Compositions that read host.dataset.variableValues directly keep working — the host attribute is still set as before. The new path is a strict addition.

🤖 Generated with Claude Code

@jrusso1020 jrusso1020 changed the base branch from feat/get-variables to graphite-base/601 May 4, 2026 19:41
Building on PR 1's getVariables() helper, this PR routes per-instance
values into the correct sub-composition. Same composition source can
now be embedded N times with different content via data-variable-values
on each host element.

How it works:

- compositionLoader, before injecting wrapped scripts, layers the host
  element's data-variable-values JSON over the sub-comp's declared
  defaults (its own data-composition-variables) and writes the merged
  object to window.__hfVariablesByComp[compositionId]. Skipped when
  both sides are empty so the table only grows for instances that
  actually carry values.
- compositionScoping's wrapper IIFE now takes a fourth parameter
  __hyperframes alongside the existing scoped document/gsap/window.
  The scoped __hyperframes shadows getVariables() to read from
  __hfVariablesByComp[__hfCompId], returning a fresh object each call
  so script mutations don't leak into the shared table.
- Top-level scripts (not wrapped by compositionScoping) keep using the
  unscoped window.__hyperframes.getVariables(), which reads
  data-composition-variables defaults plus the CLI override
  (window.__hfVariables) — same path as PR 1.
- readDeclaredDefaults is exported from getVariables.ts so the loader
  reuses the exact same defaults-extraction logic the helper uses for
  the top-level path.

Inline templates (no separate <html> document root) get host overrides
only — no declared defaults — since there's no separate <html> to read
data-composition-variables from. External sub-comps fetched via
data-composition-src get the full declared defaults + host overrides
merge.

Tests: 3 new compositionScoping tests covering scoped getVariables
invocation, missing-entry fallback, and mutation isolation. 5 new
compositionLoader tests covering merge order, declared-only path,
empty-skip, invalid-host-JSON resilience, and per-instance scoping
across two hosts sharing a source. 3 new getVariables tests covering
the newly-public readDeclaredDefaults. All 622 core tests green.

Docs: docs/concepts/compositions.mdx switched its sub-comp example from
hand-rolled JSON.parse(host.dataset.variableValues) to the new
__hyperframes.getVariables() pattern. data-attributes.mdx clarifies
per-instance scoping behavior.

This is PR 2 of a 4-PR stack. PR 3 adds schema validation + lint;
PR 4 ships skill / scaffold updates.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@jrusso1020 jrusso1020 force-pushed the feat/get-variables-subcomp branch from cedd32b to 75f892b Compare May 4, 2026 19:41
@graphite-app graphite-app Bot changed the base branch from graphite-base/601 to main May 4, 2026 19:42
- compositionLoader.ts: drop the redundant inline `Window` cast; the
  ambient `__hfVariablesByComp?` declaration in runtime/window.d.ts
  already covers it within the same package.
- compositionScoping.test.ts: drop the `__captured: undefined as unknown`
  initializers — `Record<string, unknown>` already permits the key, the
  init was noise.

Reuse + efficiency reviews returned clean. The scoped getVariables's
per-call Object.assign({}, scoped) is consistent with the file's
existing scoped-utility conventions (gsap proxy returns fresh bound
functions per access) and acceptable since the idiomatic usage
destructures once at script init.

All 44 touched core tests still green.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@jrusso1020 jrusso1020 force-pushed the feat/get-variables-subcomp branch from 75f892b to 1da6f45 Compare May 4, 2026 19:42
@jrusso1020 jrusso1020 merged commit da92a17 into main May 4, 2026
43 checks passed
Copy link
Copy Markdown
Collaborator Author

Merge activity

@jrusso1020 jrusso1020 deleted the feat/get-variables-subcomp branch May 4, 2026 20:05
jrusso1020 added a commit that referenced this pull request May 4, 2026
## What

Adds two lint rules + render-time validation for the variable system shipped in PR #600 + PR #601. Authors get fast feedback when JSON is malformed, declarations are missing required fields, or `--variables` values don't match the declared schema.

This is **PR 3 of a 4-PR stack** — based on `feat/get-variables-subcomp` (PR #601). Will retarget to `main` after PRs 1 + 2 merge.

## Why

The runtime today silently masks several classes of mistake:
- A typo in `data-variable-values` JSON makes the parser return `{}` and the script reads stale declared defaults, with no visible signal.
- A typo in a `data-composition-variables` declaration (missing `id`, wrong `type`) gets silently filtered out.
- `--variables '{"titel":"x"}'` instead of `"title"` renders fine and produces the wrong output.

These are exactly the cases lint + schema validation are good at catching.

## How

**Lint rules** (`packages/core/src/lint/rules/composition.ts`):
- `invalid_variable_values_json` — host's `data-variable-values` must parse as a JSON object.
- `invalid_composition_variables_declaration` — root `<html>`'s `data-composition-variables` must parse as an array of objects with `id`, `type` (one of string/number/color/boolean/enum), `label`, and `default`. Per-entry findings report which fields are missing or invalid.

Both rules read via a new `readJsonAttr` helper in `lint/utils.ts`. The existing `readAttr`'s regex `["']([^"']+)["']` truncates JSON-in-attribute values at the first internal quote — `data-variable-values='{"x":"y"}'` would capture only `{`. The new helper alternates double-vs-single-quoted branches with quote-specific char classes. A second helper `findHtmlTag` returns the `<html>` open tag (where `data-composition-variables` lives), distinct from `findRootTag` which returns the first in-body composition element.

**Render-time validation** (`packages/core/src/runtime/validateVariables.ts`):
- `validateVariables(values, declarations)` returns a structured array of `VariableValidationIssue`s — `undeclared`, `type-mismatch`, or `enum-out-of-range`. Pure / sync; works in any environment.
- `formatVariableValidationIssue` renders one-line user-facing strings for CLI output.
- Both exported from `@hyperframes/core`.

**CLI integration** (`packages/cli/src/commands/render.ts`):
- New `--strict-variables` flag. Default: print warnings, continue. Strict: print warnings, exit 1.
- New `validateVariablesAgainstProject(indexPath, values)` helper reads the project's `index.html`, pulls the declared schema via `extractCompositionMetadata`, validates the CLI payload. Uses the existing `ensureDOMParser` polyfill (same pattern as `compositions.ts`).

## Test plan

- [x] Unit tests added/updated
  - **11 `validateVariables` unit tests** — happy path, undeclared keys, type mismatches (every type), enum range, multi-issue aggregation, formatter output.
  - **11 `composition.test.ts` cases** — both lint rules: parse errors, shape errors, per-entry validation, unknown types, positive cases for valid declarations.
  - **5 `render.test.ts` cases** — `validateVariablesAgainstProject`: no-declarations, happy path, undeclared, type-mismatch, missing-file.
  - All existing tests green: core 646, cli 213.
- [x] Manual flow walkthrough
  - `--variables '{"title":"x"}'` against an index that declares `title` as string → no warnings.
  - `--variables '{"count":"three"}'` against `count: number` → warning printed, render continues.
  - Same with `--strict-variables` → exit 1 before render starts.
- [x] Documentation updated
  - `docs/packages/cli.mdx` — added `--strict-variables` flag row.

## Backwards compatibility

Fully additive. Existing compositions emit zero new lint findings (the rules only fire on malformed JSON or invalid declarations, which the runtime would have silently dropped anyway). Existing `hyperframes render` invocations behave identically without the new flag.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
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.

3 participants