Repository navigation
fix(lint): reject parser-invalid variable defaults - #5314
jrusso1020 merged 1 commit into
Conversation
jrusso1020
left a comment
jrusso1020
left a comment
There was a problem hiding this comment.
Approving. Reviewed at c61b0b03.
The gap is real on main. variablesDeclarationFindings only checks that id, type, label and default are present, while parseCompositionVariables drops any entry whose default doesn't match its type, or an enum with no options array. So {"type":"number","default":"3"} lints clean and then disappears from the parsed metadata. Calling the parser's own exported isCompositionVariable after the field checks is the right fix, because lint and the parser now share one definition and can't drift apart.
What I checked, on a local merge with current main:
composition.test.tspasses 235 of 235. With the oldcomposition.tsput back, exactly the 27 new invalid cases fail and the 7 valid controls still pass.- Since this makes lint stricter, I scanned every tracked HTML file for declarations that pass the old checks but fail the new predicate. There are 303 declarations and none are newly flagged, so no registry item or example starts failing lint.
Need: yes. Size: ok. Reuse: reuses isCompositionVariable from @hyperframes/parsers/composition instead of re-encoding the type table. Simpler: already minimal.
— Rames
What
Reject composition variable declarations whose defaults or enum options the existing parser cannot accept.
Why
parseCompositionVariablesdiscards a number variable with"default": "3", buthyperframes lintcurrently reports no declaration error. The same mismatch affects boolean and string-valued defaults, and enum declarations without an options array. A composition can pass lint while its variable disappears from the parsed metadata consumed by the editor and SDK.Related work
This is a small correction to the existing declaration rule. I checked all 127 current open PRs by changed path and relevant all-state searches. #4931 and #4933 also edit the composition lint files for chapter markers and crop packs; neither changes variable validation. #4391 concerns declaration placement, which current main already checks. This PR retains that behavior and does not attempt to implement those scopes.
How
Use the parser's existing
isCompositionVariablepredicate after the current required-field checks, retaining their diagnostics. Report the existinginvalid_composition_variables_declarationerror with the entry index and a corrective hint. Parser, runtime, schemas, and default values are unchanged.Test plan
lint --jsonon a local number-variable fixture emits the declaration error and exits 1.Native reproduction on unchanged production: the frozen composition test file had 27 assertion failures and 198 passes. After the fix, the full lint suite passed 925 tests in 20 files using the same frozen tests.
Also passed: full workspace build before and after; lint package typecheck; repository lint and format checks; test reachability; comment citation/share checks; normal pre-commit lint, format, typecheck, Fallow audit, tracked-artifact, large-file and commit-message hooks. Commit is SSH signed and GitHub reports Verified.
Hosted CI, browser/render regressions, and maintainer review are pending. No render or live-provider claim is made from the lint/CLI checks.