Split and combine packages, improve DX across packages #97
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Caution Review failedThe pull request is closed. WalkthroughThis update migrates all apps and packages from the legacy Changes
Sequence Diagram(s)sequenceDiagram
participant UI as App UI (Browser/Discord/Web)
participant Runtimes as @evaluate/runtimes
participant Execute as @evaluate/execute
participant Logger as @evaluate/logger
participant Fetch as @evaluate/fetch
participant API as Remote Execution API
UI->>Runtimes: fetchAllRuntimes() / searchForRuntimes()
Runtimes->>Fetch: fetch(Piston API)
Fetch-->>Runtimes: runtimes[]
Runtimes-->>UI: runtimes[]
UI->>Execute: executeCode(runtime, options)
Execute->>Fetch: fetch(API endpoint, payload)
Fetch-->>Execute: execution result
Execute-->>UI: [result, options]
UI->>Logger: createLogger().log(event)
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
apps:browser-extension
apps:discord-bot
apps:website
dependencies
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (1)
apps/discord-bot/src/events/application-authorised.ts (1)
6-6: Same alias assurance for~/helpers/session-contextAs above, make sure the path alias is present; otherwise builds will fail only in this file.
🧹 Nitpick comments (8)
biome.json (1)
25-26: Disabling two a11y rules weakens baseline accessibilityTurning off
useSemanticElementsandnoStaticElementInteractionsremoves safeguards that normally catch keyboard-navigation and semantics regressions.
If the rules are too noisy, consider narrowing their scope with overrides orallowlists rather than a blanket disable.packages/fetch/package.json (1)
1-14: Provide an explicitmain/moduletarget or rely solely on `"exports"?Node ≤16 and some tooling still fall back to
"main"when"exports"is present but empty forrequirecalls. If CJS support is intentionally dropped, note this in the README; otherwise add:"exports": { ".": { "import": "./dist/index.js", "types": "./dist/index.d.ts" + ,"require": "./dist/index.cjs" } }, + "main": "./dist/index.cjs",Also remember to add any runtime deps (
cross-fetch, etc.) once implementation lands.apps/website/src/style.css (1)
1-1: Confirm style package export name
@evaluate/stylereplaces@evaluate/styles; ensure the new package actually re-exports a root CSS entry (some bundlers require an explicit"style"export in the package’spackage.json).apps/browser-extension/src/content-script/execution/result.tsx (1)
49-79: Simplify conditional rendering logic.The
options.codecheck is redundant - you can wrap both buttons in a single conditional block since they both requireoptions.code.<div className="flex justify-end gap-2"> - {options.code && ( + {options.code && ( + <> <Button variant="secondary" asChild> <a target="_blank" rel="noreferrer noopener" href={new URL( makePickRuntimePathname(options), env.VITE_PUBLIC_WEBSITE_URL, ).toString()} > <span>Change Runtime</span> <ExternalLinkIcon size={16} className="ml-1" /> </a> </Button> - )} - - {options.code && runtime && ( + {runtime && ( <Button variant="secondary" asChild> <a target="_blank" rel="noreferrer noopener" href={new URL( makeEditCodePathname(runtime, options), env.VITE_PUBLIC_WEBSITE_URL, ).toString()} > <span>Edit Code</span> <ExternalLinkIcon size={16} className="ml-1" /> </a> </Button> + )} + </> )} </div>packages/runtimes/src/getters.ts (2)
79-85: Consider making the icon base URL configurable.The icon URL is hardcoded to a specific commit. While this ensures stability, it might be better to make this configurable or use a stable branch reference.
+const ICON_BASE_URL = process.env.ICON_BASE_URL || 'https://raw.githubusercontent.com/PKief/vscode-material-icon-theme/940f2ea7a6fcdc0221ab9a8fc9454cc585de11f0/icons'; + export function getRuntimeIconUrl(id: string) { if (!isRuntimeId(id)) return null; const slug = RuntimeIcons[id]; if (!slug) return null; - return `https://raw.githubusercontent.com/PKief/vscode-material-icon-theme/940f2ea7a6fcdc0221ab9a8fc9454cc585de11f0/icons/${slug}.svg`; + return `${ICON_BASE_URL}/${slug}.svg`; }
88-92: Fix typo in function name.The function name should follow camelCase convention with 'Id' capitalised.
-export function getRuntimeidFromExtension(extension: string) { +export function getRuntimeIdFromExtension(extension: string) {packages/execute/src/shapes.ts (2)
42-44: Consider improving the error message for clarity.The current error message could be more descriptive.
.refine((o) => o.entry in o.files, { - error: 'Entry file does not exist', + message: 'Entry file must exist in the files map', });
113-113: Consider clarifying the output selection logic.The condition
(r.compile?.code || 0) > 0might be unclear. Consider making it more explicit.- output: (r.compile?.code || 0) > 0 ? r.compile?.output : r.run?.output, + output: r.compile && r.compile.code !== 0 ? r.compile.output : r.run.output,This makes it clearer that we're checking for a non-zero compile exit code (indicating compilation failure).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (73)
apps/browser-extension/package.json(1 hunks)apps/browser-extension/src/background/index.ts(4 hunks)apps/browser-extension/src/content-script/execution/dialog.tsx(3 hunks)apps/browser-extension/src/content-script/execution/index.tsx(2 hunks)apps/browser-extension/src/content-script/execution/result.tsx(2 hunks)apps/browser-extension/src/content-script/index.tsx(2 hunks)apps/browser-extension/src/helpers/make-url.ts(0 hunks)apps/browser-extension/src/services/posthog.ts(1 hunks)apps/browser-extension/src/style.css(1 hunks)apps/browser-extension/src/styles.css(0 hunks)apps/discord-bot/package.json(1 hunks)apps/discord-bot/src/commands/evaluate.ts(3 hunks)apps/discord-bot/src/components/edit-evaluation-button.ts(1 hunks)apps/discord-bot/src/components/evaluate-modal.ts(2 hunks)apps/discord-bot/src/components/open-evaluation-button.ts(1 hunks)apps/discord-bot/src/env.ts(1 hunks)apps/discord-bot/src/events/application-authorised.ts(1 hunks)apps/discord-bot/src/handlers/evaluate.ts(5 hunks)apps/discord-bot/src/services/posthog.ts(1 hunks)apps/discord-bot/src/utilities/make-url.ts(0 hunks)apps/website/next.config.js(2 hunks)apps/website/package.json(1 hunks)apps/website/src/app/(editor)/playgrounds/[playground]/page.tsx(3 hunks)apps/website/src/app/(editor)/playgrounds/[playground]/wrapper/mobile.tsx(2 hunks)apps/website/src/app/(playgrounds)/playgrounds/page.tsx(2 hunks)apps/website/src/app/(playgrounds)/playgrounds/playground-card-list.tsx(2 hunks)apps/website/src/app/(playgrounds)/playgrounds/playground-card.tsx(4 hunks)apps/website/src/app/layout.tsx(1 hunks)apps/website/src/app/sitemap.ts(2 hunks)apps/website/src/components/editor/execute-bar/index.tsx(6 hunks)apps/website/src/components/editor/index.tsx(1 hunks)apps/website/src/components/explorer/use.tsx(4 hunks)apps/website/src/components/terminal/index.tsx(2 hunks)apps/website/src/components/terminal/use.tsx(1 hunks)apps/website/src/services/posthog.ts(1 hunks)apps/website/src/style.css(1 hunks)biome.json(1 hunks)packages/components/package.json(1 hunks)packages/engine/package.json(0 hunks)packages/engine/readme.md(0 hunks)packages/engine/src/execute/fetch.ts(0 hunks)packages/engine/src/execute/index.ts(0 hunks)packages/engine/src/runtimes/data/extensions.json(0 hunks)packages/engine/src/runtimes/fetch.ts(0 hunks)packages/engine/src/runtimes/getters.ts(0 hunks)packages/engine/src/runtimes/identifier.ts(0 hunks)packages/execute/package.json(1 hunks)packages/execute/src/arguments.ts(1 hunks)packages/execute/src/fetch.ts(1 hunks)packages/execute/src/index.ts(1 hunks)packages/execute/src/shapes.ts(1 hunks)packages/execute/src/url.ts(1 hunks)packages/fetch/package.json(1 hunks)packages/fetch/src/index.ts(2 hunks)packages/helpers/package.json(0 hunks)packages/helpers/readme.md(0 hunks)packages/helpers/src/better-fetch/index.ts(0 hunks)packages/helpers/src/create-logger/index.ts(0 hunks)packages/logger/package.json(1 hunks)packages/logger/src/index.ts(1 hunks)packages/runtimes/package.json(2 hunks)packages/runtimes/src/data/extensions.json(1 hunks)packages/runtimes/src/fetch.ts(1 hunks)packages/runtimes/src/getters.ts(1 hunks)packages/runtimes/src/index.ts(1 hunks)packages/runtimes/src/shapes.ts(1 hunks)packages/runtimes/tsconfig.json(1 hunks)packages/shapes/readme.md(0 hunks)packages/shapes/src/execute.ts(0 hunks)packages/shapes/src/index.ts(0 hunks)packages/shapes/src/runtime.ts(0 hunks)packages/style/package.json(1 hunks)pnpm-workspace.yaml(1 hunks)
💤 Files with no reviewable changes (19)
- packages/helpers/readme.md
- packages/shapes/readme.md
- apps/browser-extension/src/styles.css
- packages/helpers/src/create-logger/index.ts
- packages/engine/src/execute/index.ts
- packages/engine/readme.md
- packages/helpers/src/better-fetch/index.ts
- packages/shapes/src/index.ts
- packages/helpers/package.json
- packages/engine/package.json
- packages/engine/src/runtimes/data/extensions.json
- apps/browser-extension/src/helpers/make-url.ts
- packages/engine/src/execute/fetch.ts
- packages/engine/src/runtimes/identifier.ts
- packages/engine/src/runtimes/fetch.ts
- apps/discord-bot/src/utilities/make-url.ts
- packages/shapes/src/runtime.ts
- packages/shapes/src/execute.ts
- packages/engine/src/runtimes/getters.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: apteryxxyz
PR: apteryxxyz/Evaluate#95
File: apps/browser-extension/vite-plugins.ts:50-86
Timestamp: 2025-07-16T23:10:36.498Z
Learning: User apteryxxyz prefers if statements without braces when there's a single statement, formatting them as:
if (condition)
statement;
instead of using curly braces around the statement.
packages/execute/package.json (1)
Learnt from: apteryxxyz
PR: apteryxxyz/Evaluate#95
File: apps/browser-extension/vite-plugins.ts:69-69
Timestamp: 2025-07-16T23:10:33.941Z
Learning: In the Evaluate codebase, apteryxxyz prefers to let errors throw naturally in internal Vite plugins rather than adding explicit error handling, as it's acceptable for development tooling to fail fast and surface errors clearly.
🧬 Code Graph Analysis (13)
apps/website/src/app/sitemap.ts (1)
packages/runtimes/src/fetch.ts (1)
fetchAllRuntimes(14-39)
apps/website/src/app/(playgrounds)/playgrounds/page.tsx (1)
packages/runtimes/src/fetch.ts (1)
fetchAllRuntimes(14-39)
apps/website/src/app/(playgrounds)/playgrounds/playground-card-list.tsx (1)
packages/runtimes/src/shapes.ts (2)
Runtime(3-10)Runtime(11-11)
apps/website/src/components/terminal/index.tsx (1)
packages/runtimes/src/shapes.ts (2)
Runtime(3-10)Runtime(11-11)
apps/browser-extension/src/content-script/execution/index.tsx (2)
packages/runtimes/src/shapes.ts (2)
Runtime(3-10)Runtime(11-11)packages/execute/src/url.ts (1)
makePickRuntimePathname(9-12)
apps/website/next.config.js (1)
packages/runtimes/src/fetch.ts (1)
fetchAllRuntimes(14-39)
packages/fetch/src/index.ts (1)
packages/fetch/src/http-error.ts (1)
HttpError(4-16)
apps/browser-extension/src/content-script/execution/dialog.tsx (1)
packages/runtimes/src/shapes.ts (2)
Runtime(3-10)Runtime(11-11)
apps/website/src/app/(editor)/playgrounds/[playground]/page.tsx (1)
packages/runtimes/src/fetch.ts (2)
fetchAllRuntimes(14-39)fetchRuntimeById(60-63)
apps/website/src/components/editor/index.tsx (1)
packages/runtimes/src/shapes.ts (2)
Runtime(3-10)Runtime(11-11)
apps/website/src/app/(playgrounds)/playgrounds/playground-card.tsx (3)
packages/runtimes/src/shapes.ts (2)
Runtime(3-10)Runtime(11-11)packages/runtimes/src/getters.ts (1)
getRuntimeIconUrl(80-85)packages/components/src/card.tsx (1)
CardDescription(76-76)
packages/execute/src/url.ts (3)
packages/execute/src/shapes.ts (6)
CodeOptions(6-27)CodeOptions(29-29)FilesOptions(31-44)FilesOptions(46-46)ExecuteOptions(48-86)ExecuteOptions(88-88)packages/execute/src/compress.ts (1)
compress(11-15)packages/runtimes/src/shapes.ts (2)
Runtime(3-10)Runtime(11-11)
apps/website/src/components/explorer/use.tsx (5)
packages/runtimes/src/shapes.ts (2)
Runtime(3-10)Runtime(11-11)apps/website/src/hooks/hash-fragment.ts (1)
useHashFragment(19-37)packages/runtimes/src/getters.ts (1)
getRuntimeExamples(74-77)packages/execute/src/compress.ts (2)
compress(11-15)decompress(22-25)packages/execute/src/shapes.ts (4)
ExecuteOptions(48-86)ExecuteOptions(88-88)FilesOptions(31-44)FilesOptions(46-46)
🔇 Additional comments (85)
pnpm-workspace.yaml (1)
7-8:allowUnusedPatchesmight not be a recognisedpnpm-workspace.yamlfield – please verify
pnpm-workspace.yamlofficially supports just a handful of top-level keys (packages,dependenciesMeta,neverBuiltDependencies, etc.). I’m not finding any documentation thatallowUnusedPatchesis parsed from this file. If the intent is to suppress errors about stale patch files, the supported mechanism (as of pnpm 8/9) is an.npmrcentry:allow-unused-patches = trueor using the CLI flag
--allow-unused-patches.Placing it here may be silently ignored, leaving the problem unresolved. Could you double-check with the current pnpm docs or run
pnpm config get allow-unused-patchesto confirm?apps/website/src/services/posthog.ts (1)
1-1: Confirm@evaluate/loggerexposes a default exportThe previous helper exposed
createLoggeras a named export. Please double-check that the new@evaluate/loggerpackage ships a default export or update the import toimport { createLogger } from …to avoid a runtimeundefined is not a functionerror in browsers that don’t fail fast at build-time.packages/style/package.json (1)
2-8: Package rename: publish & migration checklistRenaming to
@evaluate/styleand moving the CSS entry point means:
- Consumers must update their
package.jsonand anyimport '@evaluate/styles/...statements; ensure every downstream package was migrated (CI search recommended).- If the old name was already published, publish a stub package that re-exports the new one to prevent breaking external users.
No action needed if the lib is private.
packages/components/package.json (1)
7-8: Path now points tostyle.css– verify the file actually exists
"./src/style.css"must be generated or copied to the package root duringnpm publish; otherwise downstream bundlers will error at resolve time.
CI can catch this with a simplenpm pack && tar -tfcheck.apps/website/src/app/layout.tsx (1)
9-9: No residualstyles.cssimports foundA repo-wide
rg --context 1 "styles\.css"only returned external references tosonner/dist/styles.css(in your CSS and a commented TSX import), so there are no leftover imports of the old localstyles.css.apps/discord-bot/src/components/open-evaluation-button.ts (1)
3-3: Alias mapping for “~/helpers/resolve-emoji” confirmed• tsconfig.base.json defines
"paths": { "~/*": ["src/*"] }
so~/helpers/resolve-emojiresolves tosrc/helpers/resolve-emoji.
• The TypeScript transform plugin (typescript-transform-paths) is enabled in tsconfig, ensuring imports are rewritten at compile time.
• No Vite configuration exists in apps/discord-bot (this is a Node/TS-built package), so bundler-level aliases aren’t required.apps/discord-bot/src/env.ts (1)
2-2: Confirm default export inzod/v4
I wasn’t able to locate a localnode_modules/zodinstall to verify, so please ensure your dependencies are installed and then double-check that themoduleormainentry innode_modules/zod/package.jsonactually points to a build that usesexport default …or
module.exports = …If it doesn’t,
import z from 'zod/v4'will fail at compile‐time (TS1192) and at runtime.
• Verify themoduleandmainfields innode_modules/zod/package.json
• Inspect the target files for a default export or CommonJS export
• Otherwise switch to the named import:import { z } from 'zod';apps/discord-bot/src/services/posthog.ts (1)
1-1: Import update LGTMSwitching to
@evaluate/loggerkeeps the file in sync with the new package split. Assuming@evaluate/logger’s default export really iscreateLogger, nothing else to flag.apps/browser-extension/src/services/posthog.ts (1)
1-1: Consistent logger import – looks goodMirrors the Discord-bot change. No further action required.
apps/discord-bot/src/components/edit-evaluation-button.ts (1)
2-4: Verify new helper pathsJust sanity-check that the moved helpers (
~/helpers/...) are re-exported with the exact same names; otherwise the class will fail at runtime.apps/website/src/components/terminal/use.tsx (1)
3-3: Type import updated correctlyImporting
ExecuteResultfrom@evaluate/executealigns with the package split. No issues spotted.packages/runtimes/tsconfig.json (1)
1-1: LGTM! Clean and appropriate TypeScript configuration.The minimal configuration extending from the base tsconfig is perfect for a new package.
apps/website/src/app/sitemap.ts (2)
3-3: Correct migration to the new runtimes package.The import change from
@evaluate/engine/runtimesto@evaluate/runtimesaligns with the package restructuring.
27-27: Function name change improves clarity.The change from
fetchRuntimes()tofetchAllRuntimes()is more descriptive and clearly indicates that all available runtimes are being fetched.apps/website/src/app/(playgrounds)/playgrounds/page.tsx (2)
1-1: Consistent migration to the new runtimes package.The import change follows the same pattern as other migrated files, correctly updating from
@evaluate/engine/runtimesto@evaluate/runtimes.
10-10: Function call correctly updated.The change from
fetchRuntimes()tofetchAllRuntimes()maintains the same functionality while using the new API.apps/browser-extension/src/style.css (1)
1-4: Well-structured style imports for the browser extension.The imports correctly aggregate styles from the updated style package, component library, and external dependencies. The
@sourcedirective appropriately sets the source directory.packages/runtimes/src/index.ts (1)
3-3: Completes the public API for the runtimes package.Adding the export for shapes functionality correctly consolidates all runtime-related exports in one place, supporting the migration from the deprecated
@evaluate/shapespackage.packages/execute/src/index.ts (1)
1-4: Clean and well-structured entry point.The re-export pattern is appropriate for a package entry point, and the use of
.jsextensions aligns with ES module standards. The modular structure with separate concerns (compress, fetch, shapes, url) promotes maintainability.apps/browser-extension/package.json (1)
17-20: Dependency migration looks good.The updated package names provide clearer separation of concerns and align with the modular architecture refactoring described in the PR objectives.
apps/website/package.json (1)
16-20: Consistent dependency migration across apps.The dependency updates match the browser extension changes, maintaining consistency across the monorepo during the modular architecture migration.
apps/browser-extension/src/content-script/index.tsx (3)
3-3: Appropriate removal of inline Sonner CSS.Commenting out the Sonner CSS import suggests it's now handled globally through the new style package, which is a cleaner approach than inline imports.
7-7: Correct import path update.The import path change from
styles.csstostyle.cssaligns with the package rename from@evaluate/stylesto@evaluate/style.
36-36: Consistent removal of Sonner styling.Commenting out the Sonner CSS reference in the styles object is consistent with the import change and maintains clean separation of concerns.
apps/website/src/components/terminal/index.tsx (2)
25-25: Type annotation correctly updated.The props interface type change from
PartialRuntimetoRuntimeis consistent with the import change and maintains type safety.
11-11: Runtime type migration complete and verifiedNo remaining references to
PartialRuntimewere found across.tsand.tsxfiles. The update fromPartialRuntimetoRuntimeinapps/website/src/components/terminal/index.tsxaligns with the migration to@evaluate/runtimes, and all existing usages (e.g.runtime.id) remain valid. Approving these changes.apps/website/next.config.js (1)
3-3: Migration to new runtime package looks good.The import update from
@evaluate/engine/runtimesto@evaluate/runtimesand the corresponding function name change fromfetchRuntimestofetchAllRuntimesis consistent with the broader package restructuring.Also applies to: 18-18
apps/website/src/app/(playgrounds)/playgrounds/playground-card-list.tsx (1)
15-15: Type migration to new runtime package is correct.The update from
PartialRuntimetoRuntimetype from@evaluate/runtimesmaintains compatibility as all used properties (id,name,aliases,tags,popularity) are present in the new type definition.Also applies to: 31-31
apps/discord-bot/src/components/evaluate-modal.ts (2)
9-9: Import path reorganization looks correct.The update from
~/utilities/session-contextto~/helpers/session-contextreflects the codebase reorganization.
42-42: handleEvaluating signature is consistent across the codebase
All calls to the updatedhandleEvaluating(interaction, runtimeResolvable, options)match its definition insrc/handlers/evaluate.ts. No mismatches or remaining call sites with the old signature were found.• Definition:
export async function handleEvaluating(interaction, runtimeResolvable: string, options: CodeOptions)(apps/discord-bot/src/handlers/evaluate.ts)
• Calls:
– apps/discord-bot/src/components/evaluate-modal.ts:handleEvaluating(submit, runtime, { code, args, input })
– apps/discord-bot/src/commands/evaluate.ts:handleEvaluating(use, runtime, { code, args, input })apps/discord-bot/package.json (1)
21-29: Dependency migration to new package architecture is correct.The removal of legacy packages (
@evaluate/engine,@evaluate/helpers,@evaluate/shapes) and addition of new modular packages (@evaluate/execute,@evaluate/logger,@evaluate/runtimes) aligns with the broader restructuring. The change to caret version ranges for external dependencies (@buape/carbon,zod) is also a good practice.packages/runtimes/package.json (1)
2-2: Package rename from shapes to runtimes is consistent with migration.The name change from
@evaluate/shapesto@evaluate/runtimescorrectly reflects the package's new focused responsibility for runtime management.packages/execute/src/arguments.ts (1)
1-1: Removal of longShortStrategy has no downstream impact – safe to proceed.
- No occurrences of
longShortStrategyfound elsewhere in the repo.- No references to “unordered” parsing patterns detected.
parseArgumentsis only used in packages/execute/src/fetch.ts, so the change is isolated.Ensure that the default (ordered-only) parsing behaviour fulfils all existing requirements; otherwise, no further action is needed.
apps/website/src/components/editor/index.tsx (1)
6-6: Confirm data-fetching functions return fullRuntimeshape
The<Editor>and<Terminal>components now require the strictRuntimetype (fields:id,version,name,aliases,popularity,tags) instead of the oldPartialRuntime. Please verify that the data-fetching functions you’re using—namelyfetchAllRuntimesandfetchRuntimein
apps/website/src/app/(editor)/playgrounds/[playground]/page.tsx—indeed return objects with all of those required fields.• Affected call sites:
– apps/website/src/app/(editor)/playgrounds/[playground]/page.tsx
tsx const runtimes = await fetchAllRuntimes(); … <Editor runtime={runtime} /> <Terminal runtime={runtime} />
• Runtime type definition for reference:
– packages/runtimes/src/shapes.tsapps/browser-extension/src/content-script/execution/index.tsx (2)
2-3: LGTM! Package migration aligned with codebase refactoring.The import and type updates correctly align with the migration from
@evaluate/shapesand@evaluate/engineto the new@evaluate/runtimesand@evaluate/executepackages. The stricterRuntimetype ensures consistent data structure.Also applies to: 6-6, 11-11
20-23: Environment variable VITE_PUBLIC_WEBSITE_URL is correctly defined and validated
- Located in apps/browser-extension/src/env.ts (via
createEnv) with a Zodz.url().transform(v ⇒ new URL(v))schema- Runtime import (
import.meta.env.VITE_PUBLIC_WEBSITE_URL) ensures the value is passed through to the extensionmakePickRuntimePathnamesimply returns a path fragment and is used correctly in thenew URL(...)callNo further action required.
packages/execute/package.json (1)
1-25: Confirm the rationale for using the scoped lexure forkIt appears that
npm:@apteryxxyz/lexure@^0.17.0is a community-maintained fork of the originallexure(which hasn’t been updated since 2020) and likely includes bug fixes or enhancements. Please:
- Verify which specific modifications or fixes this fork provides and whether they’re required by
@evaluate/execute.- Document the reasons for choosing this fork, or consider migrating to the official
lexurepackage if it meets your needs.apps/website/src/app/(editor)/playgrounds/[playground]/wrapper/mobile.tsx (1)
23-29: LGTM! Event isolation improves mobile user experience.The addition of
stopPropagation()event handlers correctly prevents unintended sheet closures when users interact with the explorer and terminal content areas. This is a solid UX improvement for mobile interactions.Also applies to: 41-47
apps/browser-extension/src/content-script/execution/dialog.tsx (3)
14-15: LGTM! Package migration aligns with new modular architecture.The import changes properly migrate from the deprecated
@evaluate/shapespackage to the new focused packages (@evaluate/executefor execution types and@evaluate/runtimesfor runtime types). This improves separation of concerns and package modularity.
29-29: Good migration to stricter Runtime type.The change from
PartialRuntime[]toRuntime[]improves type safety by using the complete runtime schema from@evaluate/runtimes. This ensures all required fields (id, version, name, aliases, popularity, tags) are present, which aligns with the new modular architecture.
72-72: Correct adaptation to updated ResultDialog API.The change to pass
options={{ code }}instead ofcodedirectly aligns with the updatedResultDialogcomponent signature from the new execution package. This maintains the same functionality while conforming to the new component interface.packages/runtimes/src/shapes.ts (4)
3-11: Well-structured Runtime schema definition.The Runtime schema provides a comprehensive model with appropriate types for all necessary runtime metadata:
- Unique identification (
id,version,name)- Discoverability (
aliases,tags)- Ranking capability (
popularity)The use of zod for validation and type inference ensures type safety throughout the codebase.
13-19: Appropriate schema for external API data.The PistonRuntime schema correctly models the external Piston API response structure with optional
runtimefield and required language metadata. This provides a clear separation between external API data format and internal Runtime representation.
11-11: Proper zod type inference pattern.The type exports using
z.infer<typeof Schema>follow zod best practices, providing both runtime validation schemas and compile-time TypeScript types for consumers.Also applies to: 19-19
1-1: Import path ‘zod/v4’ is valid.The zod 4 subpath is officially exported in the package.json (see
exports["./v4"]), soimport z from 'zod/v4'will resolve correctly. No changes required.packages/logger/src/index.ts (2)
31-36: Good API simplification with default export.Converting to a default export and inlining the options type simplifies the package interface while maintaining full functionality. The updated JSDoc clearly documents the purpose and the inline type definition reduces API surface area.
37-42: Implementation correctly preserved.The function logic remains unchanged, ensuring existing functionality is maintained while benefiting from the simplified API structure.
apps/website/src/app/(editor)/playgrounds/[playground]/page.tsx (4)
1-1: Proper migration to new runtimes package.The import update correctly migrates to
@evaluate/runtimeswith more descriptive function names (fetchAllRuntimes,fetchRuntimeById). The memoized implementations should improve performance compared to the previous functions.
13-13: Correct function call update in generateStaticParams.The change to
fetchAllRuntimes()maintains the same functionality while using the new memoized implementation, which should improve build performance when generating static params.
19-19: Appropriate function call update in generateMetadata.The change to
fetchRuntimeById()correctly maintains the same parameter passing and error handling while benefiting from the memoized implementation.
32-32: Consistent function call update in EditorPage.The change to
fetchRuntimeById()maintains consistency with the generateMetadata function and benefits from memoization when the same runtime is accessed multiple times during rendering.packages/fetch/src/index.ts (6)
3-3: Good practice to avoid shadowing global fetch.Caching
globalThis.fetchasFETCHprevents shadowing issues when the exported function is also namedfetch, ensuring the original global API is always used internally.
5-5: Improved function signature aligns with native fetch.The default export and parameter names (
input,init) now match the native fetch API signature exactly, making this a proper drop-in replacement with enhanced retry logic.
9-9: Correct usage of cached fetch reference.Using
FETCH(input, init)correctly avoids shadowing while maintaining the same functionality with the updated parameter names.
29-31: Better structured logging for retry attempts.The simplified warning message with structured parameter logging (
{ input, init }) provides better debugging information and follows modern logging practices.
32-32: Minor style improvement with shortened parameter name.The shortened callback parameter
ris a common pattern for simple Promise resolvers and maintains readability.
39-39: Improved error context with response as cause.Using the
responseobject as the error cause provides more useful debugging information (status, headers, etc.) compared to the generic error, especially for HTTP-related failures.packages/runtimes/src/data/extensions.json (2)
1-88: JSON structure looks well-organised and comprehensive.The file provides a comprehensive mapping of runtime identifiers to their associated file extensions, supporting the migration from the legacy engine package structure.
28-28: Empty string extension for “file” is intentional and correctly handledThe empty-string entry in extensions.json is used by getRuntimeidFromExtension to map files without any extension to the “file” runtime. Since Array.includes only matches exact elements:
- Only the “file” runtime has
""in its extensions array, so there’s no ambiguity.- No other code paths assume a non-empty extension for this runtime.
- fetch.ts and getRuntimeDefaultFileName work off example filenames, not extensions.
No further changes required.
apps/discord-bot/src/commands/evaluate.ts (3)
8-8: Import update aligns with package migration.The import change from
@evaluate/engine/runtimesto@evaluate/runtimescorrectly reflects the new modular package structure.
50-50: Runtime function signatures verifiedBoth
searchForRuntimesandfetchAllRuntimesinpackages/runtimes/src/fetch.tsare exported as memoized async functions and correctly returnPromise<PistonRuntime[]>, matching the original behaviour and return types. No further changes required.
73-73: Verified handleEvaluating signature matchThe handler in apps/discord-bot/src/handlers/evaluate.ts is declared as:
export async function handleEvaluating( interaction: CommandInteraction | ModalInteraction, runtimeResolvable: string, options: CodeOptions, ) { … }and the call in apps/discord-bot/src/commands/evaluate.ts:
return handleEvaluating(use, runtime, { code, args, input });correctly passes the
interaction,runtimeResolvableandoptionsparameters. No changes required.packages/execute/src/fetch.ts (2)
53-62: Excellent timeout and error handling implementation.The AbortController usage for request timeout and the error handling with cause attachment are well implemented.
22-29: Good file normalisation logic.The logic to rename
'file.code'to the runtime's default filename and set the entry point is well thought out and handles edge cases properly.apps/website/src/app/(playgrounds)/playgrounds/playground-card.tsx (4)
10-10: Good migration to centralised runtime utilities.The import change from
PartialRuntimetoRuntimeand addition ofgetRuntimeIconUrlaligns well with the new package structure and improves type safety.
30-30: Improved type safety with stricter Runtime type.Using the
Runtimetype instead ofPartialRuntimeprovides better type safety and ensures all required runtime properties are available.
60-60: Excellent simplification of icon URL handling.Replacing the local helper with
getRuntimeIconUrl(runtime.id)centralises the logic and maintains proper fallback behaviour with the empty string default.
72-72: Good simplification of version display.The change from
runtime.versions.at(-1)!toruntime.versionsimplifies the version access and aligns with the new Runtime schema that has a single version property.packages/execute/src/url.ts (3)
9-12: Well-designed pathname generation for runtime selection.The
makePickRuntimePathnamefunction correctly normalises options and compresses state for the playground URL. The pathname-only approach provides good flexibility for different base URLs.
14-20: Excellent code execution URL generation.The
makeEditCodePathnamefunction properly incorporates the runtime ID into the pathname and maintains consistent state compression. Theas constreturn type provides good type safety.
1-7: Good modular imports and type usage.The imports correctly reference the new package structure and utilise the appropriate types from the shapes module.
apps/browser-extension/src/content-script/execution/result.tsx (1)
54-57: Add error handling for URL construction.The URL construction could throw if
env.VITE_PUBLIC_WEBSITE_URLis not a valid URL. Consider wrapping in a try-catch or validating the base URL.+const baseUrl = env.VITE_PUBLIC_WEBSITE_URL; +if (!baseUrl) { + console.error('Website URL not configured'); + return null; // Or handle appropriately +} href={new URL( makePickRuntimePathname(options), - env.VITE_PUBLIC_WEBSITE_URL, + baseUrl, ).toString()}Also applies to: 70-73
⛔ Skipped due to learnings
Learnt from: apteryxxyz PR: apteryxxyz/Evaluate#95 File: apps/browser-extension/vite-plugins.ts:69-69 Timestamp: 2025-07-16T23:10:33.941Z Learning: In the Evaluate codebase, apteryxxyz prefers to let errors throw naturally in internal Vite plugins rather than adding explicit error handling, as it's acceptable for development tooling to fail fast and surface errors clearly.apps/website/src/components/editor/execute-bar/index.tsx (2)
52-57: Consider the impact of reducing the entry file selection timeout.Reducing the timeout from 1000ms to 500ms might cause issues on slower systems where files take longer to load. This could result in no entry file being selected.
Has this timeout reduction been tested across different system speeds and file counts?
61-80: Well-implemented mutation configuration.The mutation is properly configured with the new API, includes proper error handling, and captures relevant telemetry data.
apps/browser-extension/src/background/index.ts (1)
78-84: Consider error handling strategy for multiple runtime executions.Re-throwing the error after sending the message might prevent other runtime executions from completing when using
Promise.all. Consider whether you want fail-fast behaviour or to collect all results..catch((error) => { const message = error instanceof Error ? error.message : 'Unknown error'; sendMessage('executionFailed', { errorMessage: message }, endpoint); - throw error; + return { error: message }; // Return error result instead of throwing });apps/website/src/components/explorer/use.tsx (4)
4-11: Import changes look good!The migration to the new
@evaluate/executeand@evaluate/runtimespackages is correctly implemented.
34-41: Runtime integration implemented correctly.Good use of optional chaining to handle the null case from
getRuntimeExamples.
44-57: Root folder initialization logic is well-structured.Good defensive programming by checking for special files before adding them.
59-67: URL saving implementation is correct.The migration to use
encodeExplorermaintains the same functionality with the new architecture.apps/discord-bot/src/handlers/evaluate.ts (3)
11-24: Import updates align with the new package architecture.Clean migration to the new
@evaluate/executeand@evaluate/runtimespackages.
51-117: Evaluation handler successfully migrated to new API.Good error handling for runtime lookup and proper use of the new execute API that returns both result and options.
119-167: Payload creation properly adapted to new data structures.The function correctly extracts data from the new
ExecuteOptionsformat and handles special files appropriately.packages/execute/src/shapes.ts (3)
6-29: Well-designed CodeOptions schema with useful conversion method.The static
frommethod provides a clean API for converting between option formats.
48-88: Excellent implementation of ExecuteOptions with metadata calculation.The transform elegantly computes aggregate metrics, and the
frommethod provides seamless conversion from CodeOptions.
90-102: Clean implementation of execution phase results.Good use of transformations to derive meaningful flags from raw execution data.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…valuate into refactor/package-dx
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/execute/src/shapes.ts (1)
6-27: Solid implementation with minor type safety considerationThe
CodeOptionsschema and static method are well-designed. Thefrommethod correctly handles the union type conversion.Consider adding return type annotation for better type safety:
- from(value: CodeOptions | FilesOptions): CodeOptions { + from(value: CodeOptions | FilesOptions): z.infer<typeof CodeOptions> {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/website/src/components/explorer/use.tsx(4 hunks)packages/execute/src/arguments.ts(1 hunks)packages/execute/src/fetch.ts(1 hunks)packages/execute/src/shapes.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/execute/src/fetch.ts
- apps/website/src/components/explorer/use.tsx
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: apteryxxyz
PR: apteryxxyz/Evaluate#95
File: apps/browser-extension/vite-plugins.ts:50-86
Timestamp: 2025-07-16T23:10:36.498Z
Learning: User apteryxxyz prefers if statements without braces when there's a single statement, formatting them as:
if (condition)
statement;
instead of using curly braces around the statement.
🔇 Additional comments (5)
packages/execute/src/shapes.ts (4)
1-2: LGTM: Clean imports with good library choicesGood use of
es-toolkitfor utilities and standard Zod import pattern.
31-44: Excellent validation with comprehensive refinementsThe
FilesOptionsschema properly validates the structure and includes a critical refinement to ensure the entry file exists in the files object. The use ofcatchall(z.string())for additional files is well thought out.
96-107: Good transformation logic with proper boolean derivationThe
ExecutionPhaseResultschema correctly derivessuccessandexpiredflags from the raw execution data. The output trimming with fallback toundefinedis a nice touch for clean data handling.
110-122: Robust result aggregation with sound logicThe
ExecuteResultschema properly aggregates compilation and runtime results. The success condition logic (r.run.success && (!r.compile || r.compile.success)) correctly handles optional compilation phases. The output selection priority (compilation errors over runtime output) makes sense.packages/execute/src/arguments.ts (1)
1-6: Well-configured lexer with good quote supportThe lexer configuration properly supports both standard (
") and curly ("") quotes, which improves usability for different input sources.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style
Chores
@evaluate/execute,@evaluate/logger,@evaluate/runtimes,@evaluate/style).