Skip to content

Conversation

@na-trium-144
Copy link
Contributor

@na-trium-144 na-trium-144 commented Nov 17, 2025

close #106

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Nov 17, 2025

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
my-code 7ab2c46 Commit Preview URL

Branch Preview URL
Nov 18 2025, 08:47 AM

Copy link

Copilot AI left a 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 refactors the runtime initialization pattern from automatic (on component mount) to lazy initialization via a new init() method. This allows runtimes to be initialized only when needed, improving performance by avoiding unnecessary initialization of unused language runtimes.

  • Added init() method to the RuntimeContext interface for manual initialization control
  • Modified worker-based runtimes (Python, Ruby, JavaScript) to use mutex-protected lazy initialization
  • Updated TypeScript runtime to support lazy initialization with a guard flag
  • Refactored useRuntime() to call init() automatically via useEffect

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 11 comments.

Show a summary per file
File Description
app/terminal/worker/runtime.tsx Converted worker initialization from automatic useEffect to manual init() method with mutex protection and early-return guard
app/terminal/typescript/runtime.tsx Added init() method with initialization guard flag, removed AbortController cleanup logic
app/terminal/runtime.tsx Added init field to RuntimeContext interface, modified useRuntime() to call init() via useEffect, changed switch from direct returns to break statements
app/terminal/page.tsx Added manual init() call for each runtime before running tests
app/terminal/README.md Documented the new init() method, its behavior, and usage patterns

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 30 to 33
runFiles: (
filenames: string[],
files: Readonly<Record<string, string>>
) => Promise<ReplOutput[]>;
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] While the reformatting of the runFiles function signature improves readability, this appears to be purely a formatting change unrelated to the main purpose of adding init(). Consider keeping such formatting changes in a separate commit or PR to maintain clearer git history and easier code review.

Suggested change
runFiles: (
filenames: string[],
files: Readonly<Record<string, string>>
) => Promise<ReplOutput[]>;
runFiles: (filenames: string[], files: Readonly<Record<string, string>>) => Promise<ReplOutput[]>;

Copilot uses AI. Check for mistakes.
* useRuntime() 内のuseEffectで呼び出されます。
* ランタイムの初期化にコストがかかるものは、init()されたときにだけ初期化するようにします。
* useRuntime() が複数回使われた場合はinitも複数回呼ばれます。
* 初期化は非同期に行い、完了する前にreturnしてよいです。
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] In the documentation, line 17 states "初期化は非同期に行い、完了する前にreturnしてよいです" (initialization is done asynchronously and may return before completion). This is accurate for the TypeScript runtime's init() implementation which starts async initialization without awaiting. However, consider clarifying that the ready flag should be checked before using the runtime to ensure initialization has completed.

Suggested change
* 初期化は非同期に行い、完了する前にreturnしてよいです。
* 初期化は非同期に行い、完了する前にreturnしてよいです。
* そのため、ランタイムを利用する前に `ready` フラグが `true` であることを必ず確認してください。

Copilot uses AI. Check for mistakes.
const { init } = runtime;
useEffect(() => {
init?.();
}, [init]);
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The useEffect has init in its dependency array, but init is a callback that may change on every render depending on its dependencies. This could cause unnecessary re-initialization. Since the README documentation states that "useRuntime() が複数回使われた場合はinitも複数回呼ばれます", this behavior might be intentional, but consider whether this effect should only run once by using an empty dependency array instead.

Suggested change
}, [init]);
}, [language]);

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@na-trium-144 na-trium-144 merged commit 858ff56 into main Nov 18, 2025
3 checks passed
@na-trium-144 na-trium-144 deleted the worker-init-ondemand branch November 18, 2025 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ランタイムの初期化が遅い・通信量が重い

2 participants