-
Notifications
You must be signed in to change notification settings - Fork 4
Vendor @observablehq/runtime
#208
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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 pull request vendors the @observablehq/runtime package by rewriting it in TypeScript to enable better type-checking and code prompts for the Recho Companion project. The implementation is a complete rewrite of the reactive runtime system that manages computation graphs and variable dependencies.
Key Changes:
- Complete TypeScript implementation of the Observable runtime (Runtime, Module, Variable classes)
- Comprehensive test suite covering all runtime functionality
- Path alias configuration (
@/) for cleaner imports - Removal of
@observablehq/runtimenpm dependency
Reviewed changes
Copilot reviewed 34 out of 48 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
lib/runtime/*.ts |
New vendored TypeScript runtime implementation with core classes (Runtime, Module, Variable) and utilities |
test/runtime/**/*.spec.ts |
Comprehensive test suite for the vendored runtime functionality |
vite.config.js |
Updated to add path alias configuration and removed React-specific test setup |
package.json |
Removed @observablehq/runtime dependency and updated dev script config |
test/playground/**/* |
Updated playground components and infrastructure with new path aliases |
types/observablehq__runtime.d.ts |
Removed (no longer needed with vendored TypeScript implementation) |
runtime/index.js |
Updated import to use vendored runtime instead of npm package |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (4)
test/playground/components/BlockItem.tsx:24
- The 'shrink-0' class is the Tailwind v3+ utility, but this was previously 'flex-shrink-0'. While both work, ensure the project is using a version of Tailwind CSS that supports the shorthand 'shrink-0' utility class to avoid potential styling issues.
test/playground/components/TestSelector.tsx:3 - The import path uses "@/test/js/index.js" but based on the alias configuration, it should be "@/test/playground/js/index.js". The alias "@/" maps to the repository root, so test files should include the full path from root.
test/playground/components/App.tsx:5 - The import path uses "@/test/js/index.js" but based on the alias configuration, it should be "@/test/playground/js/index.js". The alias "@/" maps to the repository root, so test files should include the full path from root.
test/playground/components/BlockItem.tsx:29 - There's a space in "gap-0 5" which should be "gap-0.5" for proper Tailwind CSS class syntax.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pearmini
left a comment
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.
Add source to runtime and runtime tests
pearmini
left a comment
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.
LGTM!
Considering the series of operations to be performed on the future Recho Companion, and to provide better TypeScript-based code prompts, I rewrote and vendored the
@observablehq/runtimepackage with the help of AI.