-
Notifications
You must be signed in to change notification settings - Fork 191
[WIP] Begin moving towards invokable wire format #1690
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
base: main
Are you sure you want to change the base?
Conversation
|
cc230f3
to
4625e5e
Compare
a5f57d9
to
cddc4dc
Compare
6a5d569
to
87e01ac
Compare
0683762
to
9ba08a7
Compare
2b97bec
to
ecadfb7
Compare
ecadfb7
to
ccb6165
Compare
There was a problem hiding this comment.
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 begins transitioning towards an invokable wire format by updating integration test harnesses, refining event recording and cache behavior, and adding comprehensive documentation and pseudocode for the Glimmer reactivity system.
- Updated integration tests with new CSS and improved event recording handling.
- Refactored build configuration to adjust the typescript helper and updated the benchmark environment to reference layout compilation.
- Added extensive documentation and pseudocode for the reactive system, including tags, primitives, and composition.
Reviewed Changes
Copilot reviewed 171 out of 176 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
packages/@glimmer-workspace/integration-tests/lib/setup-harness.ts | Added import for new CSS tweaks. |
packages/@glimmer-workspace/integration-tests/lib/render-test.ts | Extended test interfaces and recording event functionality. |
packages/@glimmer-workspace/integration-tests/lib/modes/jit/delegate.ts | Minor whitespace adjustment. |
packages/@glimmer-workspace/integration-tests/lib/compile.ts | Updated precompileJSON call with meta information. |
packages/@glimmer-workspace/integration-tests/index.ts | Updated exports to include additional test helpers. |
packages/@glimmer-workspace/build/lib/config.js | Refactored typescript function signature and adjusted SWC settings. |
packages/@glimmer-workspace/benchmark-env/lib/benchmark/util.ts | Replaced reference to compilable with layout in entry compilation. |
guides/reactivity/* | New and updated documentation and pseudocode for the reactivity system. |
Files not reviewed (5)
- .prototools: Language not supported
- .vscode/settings.json: Language not supported
- package.json: Language not supported
- packages/@glimmer-workspace/build/package.json: Language not supported
- packages/@glimmer-workspace/integration-tests/lib/harness/tweaks.css: Language not supported
Comments suppressed due to low confidence (3)
packages/@glimmer-workspace/build/lib/config.js:410
- The typescript() function signature was changed by removing the package parameter. Please ensure all callers have been updated accordingly and update any related documentation or JSDoc to reflect this change.
typescript(env),
packages/@glimmer-workspace/benchmark-env/lib/benchmark/util.ts:5
- The property reference was changed from 'compilable' to 'layout'. Verify that 'layout' exists on entry and that this change is consistent with the intended API to avoid compilation issues.
return unwrapHandle(entry.layout!.compile(context));
packages/@glimmer-workspace/integration-tests/lib/render-test.ts:112
- [nitpick] This diff introduces private class fields (using the '#' syntax). Ensure that the target TypeScript/JavaScript environment fully supports private fields to avoid runtime errors.
#events: RecordedEvent[] = [];
Any way that I can contribute? In the context of #1666, this would be very helpful. |
idk, basically what's left here is that I need to add tests, and make sure CI goes green |
- Remove @glimmer-workspace/env from types array (workspace packages don't work in types array) - Fix type errors in @glimmer/program using isIndexable utility - Update import paths to use public APIs instead of deep imports - Add proper type guards for unknown types passed to typed functions These minimal changes resolve the TypeScript compilation errors that were blocking the build. The build now successfully compiles all packages but hits memory limits during production bundling (separate issue).
- Add tools in bin/fixes/ for applying ESLint suggestions and TypeScript code fixes - Update CLAUDE.md to document automation tools for future reference - Fix unnecessary escape characters in strict-mode-test.ts - Tools enable systematic fixing of linting issues for both humans and LLMs
- Remove unused keyword filtering variables in keyword-errors-test.ts - Remove debugger statement in verify.ts - Remove unused 'full' variable in syntax-error.ts - Add future-github-issues.md to track follow-up improvements Reduces linting errors from 51 to 48
- Fix unbound method warnings by wrapping QUnit methods in arrow functions - Remove unused TYPES variable in keyword-errors-test.ts - Remove unused RenderTestFunction interface Reduces linting errors from 48 to 42
- Fix deep import path in curry-value.ts to use public API - Remove empty block statement in normalization pass - Both changes verified with successful build Reduces linting errors from 42 to 40
- Remove unused KeywordType import - Remove unused private fields: #append, #named, #span, #container, #parent, #curly - Prefix unused parameters with underscore: _named, _span - All removed code was genuinely unused dead code Reduces linting errors from 33 to 24
- Remove unused private fields from GlimmerSyntaxError class - Replace non-null assertion with proper error checking in let statement compilation - Update deprecated blockParams API to use params for better error propagation - Mark unused constructor parameters with underscore prefix Reduces linting errors from ~51 to 17, with remaining errors being documented unsafe TypeScript operations.
…rence The original commit 86a354a "Begin moving towards invokable wire format" was incomplete: - serialize.ts expected ASTv2.ResolvedVarReference but the class was missing - isVariableReference() checked for 'Resolved' type but ResolvedVarReference didn't exist - FreeVarReference was renamed to LexicalVarReference but resolved variant was missing Changes: - Add missing ResolvedVarReference class to complete the refactor - Add AnyArgs union type for serialize functions - Add trusting property to ComponentArg class - Update supporting code to handle the restored types Resolves all 16 TypeScript unsafe operation linting errors.
- Uses existing bin/ infrastructure (chalk, execa) - Focuses on essential checks (linting) without expensive builds - Avoids TypeScript node_modules traversal issues - Provides quick validation before pushing changes
- Fix undefined checks in ESLint suggestion scripts - Add missing LanguageServiceHost methods for TypeScript codefixes - Improve error handling in automation scripts - Update ESLint configuration for executable TypeScript files - Simplify CI validation script to focus on linting
6aa74e5
to
dd2e3dd
Compare
No description provided.