-
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
- Eliminate special high-level opcodes - By modifying the statements and expressions to use the new encoder APIs, it will be easier for us to emit the encoder calls directly from the wire format - Split modifier and component to resolved variants - Clarify args wire format - Separate kinds of component invocations - Disentangle the types of helpers - Streamline components
The only remaining test failures are syntax errors that need updated test assertions.
- `strict-mode.ts` now type checks. - Next step: deriving error messages from the validation contexts.
TODO: consolidate error propagation through elements.
The only remaining failures are now outdated error message tests.
Syntax errors are now notes in the AST rather than reported during parse time, and are reported at compile-time. To make it possible to get syntax errors purely from `@glimmer/syntax`, this commit adds a verifier to `@glimmer/syntax` that validates the standalone `ASTv2` and reports errors that it finds. To get a complete picture, you really _do_ want to wait until compile-time, which can report keyword-specific errors, but this makes it possible to get a sense of the syntax errors without involving the compiler. This is all in service of making errors reported by `@glimmer/syntax` fully forgiving.
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 |
No description provided.