Move minecraft protocol into worker! #319
Conversation
|
/deploy |
|
Deployed to Vercel Preview: https://prismarine-cj5i34398-zaro.vercel.app |
|
/deploy |
|
/deploy |
|
Deployed to Vercel Preview: https://prismarine-l8h0tc7ws-zaro.vercel.app |
|
Deployed to Vercel Preview: https://prismarine-fhqbc3qqq-zaro.vercel.app |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (11)
src/protocolWorker/protocolWorkerGlobals.ts (1)
1-1: Consider removing or documenting commented codeThis commented code doesn't seem to serve a purpose. If it's important for understanding the module, please add an explanatory comment; otherwise, consider removing it.
src/protocolWorker/protocolWorker.ts (4)
14-15: Address the TODO.A TODO comment mentions using another strategy by sending all events. If this is still planned, consider creating an issue or backlog item to track it, ensuring it doesn’t get overlooked.
104-121: Avoid potential concurrency pitfalls when modifying globals.The
registeredChannelsarray is globally tracked, and updates happen in thecall(...)method. While workers run single-threaded, you may wish to encapsulate channel management within a separate class or function to avoid accidental side effects and maintain clearer state management.
123-134: Consider a standard ping approach.
pingProxymanually sends a string message and listens for a “pong” event. Ensure that incoming messages of the same format do not cause collisions or security issues. Alternatively, a standard keepalive approach may reduce the complexity of custom ping/pong logic.
298-299: Address important TODO regarding socket close events.A comment notes that the socket’s close event is never triggered. Investigate whether this stems from the underlying proxy or the environment. This may lead to resource leaks or unexpected behavior.
src/index.ts (2)
124-127: Refactor away fromreduce().The pipeline warns against
Array#reduce()at line 124 (unicorn/no-array-reduce). Consider using a simple loop,forEach, orObject.fromEntriesto constructserverSettingsQs:-const serverSettingsQs = serverSettingsQsRaw.map(x => x.split(':')).reduce<Record<string, string>>((acc, [key, value]) => { - acc[key] = JSON.parse(value) - return acc -}, {}) +const serverSettingsQsEntries = serverSettingsQsRaw.map(x => x.split(':')).map(([k, v]) => [k, JSON.parse(v)]) +const serverSettingsQs = Object.fromEntries(serverSettingsQsEntries)🧰 Tools
🪛 GitHub Actions: CI
[warning] 124-124:
Array#reduce()is not allowed unicorn/no-array-reduce
491-493: Remove the empty block or add a comment.The pipeline flags the empty
else {}statement here (no-empty). Consider removing it entirely or documenting why it is intentionally empty.} else { -} +// else condition intentionally omitted for future extension ... }🧰 Tools
🪛 GitHub Check: build-and-deploy
[failure] 491-491:
Empty block statement🪛 ESLint
[error] 491-493: Empty block statement.
(no-empty)
🪛 GitHub Actions: CI
[error] 491-491: Empty block statement no-empty
renderer/viewer/lib/workerProxy.ts (1)
1-1: Clarify the return type instead of usingvoidin a union.
The usage ofvoid | Promise<any>can be confusing and is flagged by static analysis. Consider using something likePromise<void> | Promise<any>or returningundefinedin synchronous cases to avoid ambiguity.-export function createWorkerProxy<T extends Record<string, (...args: any[]) => void | Promise<any>>> (handlers: T, channel?: MessagePort): { __workerProxy: T } { +export function createWorkerProxy<T extends Record<string, (...args: any[]) => Promise<any> | undefined>> (handlers: T, channel?: MessagePort): { __workerProxy: T } {🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/protocolWorker/protocolMain.ts (1)
52-61: Terminate the Worker explicitly when done.
You have logic to terminate the worker when the bot ends or ifbotis not available, but there’s no fallback for unexpected errors or if the client is reinitialized multiple times. Consider adding robust cleanup or referencing the Worker in a higher-level lifecycle.src/protocolWorker/microsoftAuthflow.ts (2)
20-20: Consider returning strongly-typed data structures.
The function signature returns an object with a shallow type but sets everything toany. You may benefit from specifying structured interfaces to make the code more maintainable.
115-125: Ensure the message event listener is cleaned up.
This function attaches a 'message' event listener on the worker but doesn’t remove it upon completing the auth flow. If called multiple times, it may cause memory leaks or unexpected behavior.worker.addEventListener('message', function handler({ data }) { if (data.type === 'authFlow') { authData.setConnectingVersion(data.version) resolve() + worker.removeEventListener('message', handler) } })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
.eslintrc.json(1 hunks)renderer/viewer/lib/simpleUtils.ts(1 hunks)renderer/viewer/lib/workerProxy.ts(3 hunks)renderer/viewer/lib/worldrendererCommon.ts(1 hunks)src/index.ts(9 hunks)src/mineflayer/mc-protocol.ts(1 hunks)src/mineflayer/plugins/ping.ts(1 hunks)src/optionsStorage.ts(1 hunks)src/protocolWorker/microsoftAuthflow.ts(6 hunks)src/protocolWorker/protocolMain.ts(1 hunks)src/protocolWorker/protocolWorker.ts(1 hunks)src/protocolWorker/protocolWorkerGlobals.ts(1 hunks)src/react/AppStatusProvider.tsx(1 hunks)src/shims/minecraftData.ts(1 hunks)src/utils.ts(0 hunks)
💤 Files with no reviewable changes (1)
- src/utils.ts
🧰 Additional context used
🧬 Code Graph Analysis (5)
src/protocolWorker/protocolWorker.ts (3)
renderer/viewer/lib/workerProxy.ts (1)
createWorkerProxy(1-19)src/mineflayer/websocket-core.ts (1)
getWebsocketStream(17-54)src/mineflayer/minecraft-protocol-extra.ts (1)
validatePacket(57-78)
renderer/viewer/lib/simpleUtils.ts (1)
renderer/viewer/lib/worldrendererCommon.ts (1)
version(155-157)
src/index.ts (2)
src/parseServerAddress.ts (1)
parseServerAddress(3-46)src/protocolWorker/protocolMain.ts (1)
getProtocolClientGetter(32-211)
src/protocolWorker/protocolMain.ts (7)
src/protocolWorker/protocolWorker.ts (1)
PROXY_WORKER_TYPE(36-135)src/connect.ts (1)
ConnectOptions(13-42)src/parseServerAddress.ts (1)
ParsedServerAddress(48-54)src/protocolWorker/microsoftAuthflow.ts (2)
getAuthData(20-113)authFlowMainThread(115-164)src/appStatus.ts (1)
setLoadingScreenStatus(5-35)renderer/viewer/lib/workerProxy.ts (1)
useWorkerProxy(32-76)src/appParams.ts (1)
appQueryParams(77-86)
src/protocolWorker/microsoftAuthflow.ts (5)
src/connect.ts (1)
ConnectOptions(13-42)src/react/SignInMessageProvider.tsx (1)
signInMessageState(6-12)src/react/serversStorage.ts (1)
updateAuthenticatedAccountData(82-86)src/react/NotificationProvider.tsx (1)
showNotification(22-37)src/appStatus.ts (1)
setLoadingScreenStatus(5-35)
🪛 GitHub Actions: CI
src/index.ts
[warning] 124-124: Array#reduce() is not allowed unicorn/no-array-reduce
[error] 491-491: Empty block statement no-empty
🪛 Biome (1.9.4)
renderer/viewer/lib/workerProxy.ts
[error] 1-1: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 63-63: Do not add then to an object.
(lint/suspicious/noThenProperty)
🔇 Additional comments (16)
renderer/viewer/lib/simpleUtils.ts (1)
36-41: Good implementation of the relocatedtoMajorVersionutility.The function provides a clean implementation for extracting major version information and the comment clearly indicates its limitation regarding snapshot versions.
src/optionsStorage.ts (1)
69-69: Good addition of the protocol worker configuration option.Adding
protocolWorkerOptimisation: trueas a default setting aligns with the PR objective of enhancing performance through worker-based architecture for Minecraft protocol tasks. Setting it totrueby default ensures users immediately benefit from the 10% performance improvement on high-load servers..eslintrc.json (1)
202-203: LGTM: ESLint rule addition for protocol worker compatibility.Disabling the
unicorn/relative-url-stylerule is appropriate for this codebase, especially when working with protocol-related code that may need flexibility in URL formatting.src/react/AppStatusProvider.tsx (1)
7-7: Correctly updated import path to reflect new module structure.The import path update for
getProxyDetailsaligns with the PR's architecture changes, moving Microsoft authentication flow logic to the new protocol worker module.src/protocolWorker/protocolWorkerGlobals.ts (1)
3-6: Good setup for Worker environmentAppropriate polyfills for the web worker environment. The ESLint disable is properly explained, and the TypeScript type assertion is necessary to handle the Node.js compatibility expectations in a browser environment.
src/shims/minecraftData.ts (1)
2-2: Import path update looks goodThe import path for
toMajorVersionhas been properly updated to come from the new location inrenderer/viewer/lib/simpleUtils. This aligns with the PR's objective of reorganizing utility functions.src/mineflayer/plugins/ping.ts (2)
2-2: Good addition of worker protocol importThe import of
getProtocolWorkerChannelis correctly added to support the new worker-based architecture for handling Minecraft protocol tasks.
8-11: Well-implemented worker-based ping fallbackThis code correctly implements a fallback for ping functionality when the WebSocket connection isn't available, leveraging the new worker-based architecture. The conditional check is appropriate, and the null coalescing operator handles the case when the protocol worker channel isn't available.
renderer/viewer/lib/worldrendererCommon.ts (2)
17-17: Consolidated imports correctlyThe import statement has been correctly updated to import both
chunkPosandtoMajorVersionfrom the same module, following good practices for import organization.
525-525: Successfully using the relocated utility functionThe
toMajorVersionfunction is correctly used here after its relocation to thesimpleUtilsmodule, ensuring consistent version handling throughout the application.src/mineflayer/mc-protocol.ts (1)
10-12: Reintroduce or confirm removal of packet validation logic.Previously, packet validation calls were removed in favor of simply updating
lastPacketTime. If packet validation was deliberately dropped, confirm that security and stability requirements are still met. Otherwise, consider re-adding a lightweight validation step or verifying that the trust boundary for packets is acceptable.src/protocolWorker/protocolWorker.ts (3)
1-8: Confirm Node modules availability in a Worker environment.This file imports
netfrom Node, which may not be natively available in web workers. Ensure that the bundling environment supports or mocks such modules, or consider limiting import usage to avoid runtime issues in browsers.
137-173: Evaluate user prompt flow for Microsoft authentication.This section offloads and awaits external messages to complete the MS authentication flow. Consider error handling for timeouts in case the main thread never sends
'authflowResult', preventing a potential indefinite wait.
44-103: 🛠️ Refactor suggestionCheck for lost local state with
skipWriteLog.When
data.nameis'write',skipWriteLogis set totrueand never reset tofalse. This one-time override may unintentionally disable logging for all subsequent writes.if (data.name === 'write') { - skipWriteLog = true + skipWriteLog = true + setTimeout(() => { + skipWriteLog = false + }, 0)Likely an incorrect or invalid review comment.
src/protocolWorker/protocolMain.ts (1)
78-152:❓ Verification inconclusive
Validate the usage of global window properties.
This block relies on multiple global properties likewindow.stopPacketsProcessing,window.skipPackets,window.packetTimings, etc. If they are not guaranteed to exist or be valid across different environments, it can lead to runtime errors. Ensure they are declared and maintained carefully or encapsulate them in a local state management structure.
🏁 Script executed:
#!/bin/bash # Searching for definitions of window.stopPacketsProcessing, window.skipPackets, window.packetTimings rg -A 5 'window\.(stopPacketsProcessing|skipPackets|packetTimings)'Length of output: 3437
Validate and encapsulate global window properties usage
The logic in
src/protocolWorker/protocolMain.ts(and related usage inprotocolWorker.ts) appears to rely on several global window properties which are lazily assigned (e.g.packetTimings). While the current checks (e.g. ensuringpacketTimingsexists before assignment and safe guarding array accesses forstopPacketsProcessingandskipPackets) prevent immediate runtime errors in a browser environment, please review the following points:
- Verify that all global properties (such as
stopPacketsProcessing,skipPackets,packetTimings,lastStatsUpdate, andpacketsThreadBlocking) are intentionally kept on the globalwindowobject. This is especially crucial if the code might be executed in non-browser contexts.- Consider encapsulating these properties within a dedicated state management module or object. This can improve maintainability and reduce risks associated with implicit global state reliance.
- Confirm that any external code or initialization logic guarantees that these properties are set as expected across all environments.
Please review these suggestions to ensure robust initialization and avoid any unintended side effects.
src/protocolWorker/microsoftAuthflow.ts (1)
116-146: Secure usage of tokens.
Ensure that this code never logs or accidentally discloses sensitive tokens or credentials (particularly in production). Review data flows carefully to prevent PII leakage.
|
/deploy |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rsbuild.config.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-and-deploy
🔇 Additional comments (1)
rsbuild.config.ts (1)
228-229: Good sequence adjustment for service worker generation.Adding the
patchWorkerImport()call beforegenerateSW()ensures all worker scripts have their import paths properly patched before the service worker is generated, which is essential for the protocol worker to function correctly.
|
Deployed to Vercel Preview: https://prismarine-doyllmug7-zaro.vercel.app |
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (2)
src/protocolWorker/protocol.worker.ts (1)
281-321: 🛠️ Refactor suggestionGlobal state and
net.Socketprototype modifications
wrapClientSocketupdatesnet.Socket.prototype.handleStringMessageand regards a globallastKnownKickReason. If multiple Worker instances or clients run concurrently, these shared modifications can produce race conditions.src/protocolWorker/protocolMain.ts (1)
212-212: 💡 Verification agent❓ Verification inconclusive
Address build pipeline error
CI logs indicate "NotFound('worker-rspack-loader')". Confirm thatworker-rspack-loaderis installed in dependencies and configured in your build.
🏁 Script executed:
#!/bin/bash # Checking for references to worker-rspack-loader in package.json rg 'worker-rspack-loader' package.jsonLength of output: 38
Action Required: Missing Dependency 'worker-rspack-loader'
- The search in
package.jsondid not reveal any references toworker-rspack-loader, which likely explains the CI error ("NotFound('worker-rspack-loader')").- Please confirm whether this loader is required:
- If it is needed, add it to your dependencies (e.g., via
npm install worker-rspack-loaderor the appropriate package manager) and verify that your build (rspack) configuration properly references it.- If it isn’t needed, remove or update any build configuration or code references to it in
src/protocolWorker/protocolMain.ts(line 212) and related files.
♻️ Duplicate comments (1)
rsbuild.config.ts (1)
264-277: 🛠️ Refactor suggestionThe worker import path patching has potential edge cases.
The function effectively replaces static import paths with dynamic ones based on the current URL, which is good for deployment flexibility. However, there are some concerns:
- The regex replacement is brittle and assumes a specific format for import paths
- The check for multiple
importScriptscalls may not work as expected -match?.lengthcounts capture groups, not matches- No directory existence check before trying to read from it
const patchWorkerImport = () => { + if (!fs.existsSync('./dist/static/js/async')) { + console.warn('Directory ./dist/static/js/async does not exist, skipping worker import patching') + return + } const workerFiles = fs.readdirSync('./dist/static/js/async').filter(x => x.endsWith('.js')) let patched = false for (const file of workerFiles) { const filePath = `./dist/static/js/async/${file}` const content = fs.readFileSync(filePath, 'utf8') - const match = content.match(/importScripts\([^)]+\)/) - if (match?.length > 1) throw new Error('Multiple importScripts found in ' + filePath) + const matches = content.match(/importScripts\([^)]+\)/g) || [] + if (matches.length > 1) throw new Error('Multiple importScripts found in ' + filePath) - const newContent = content.replace(/importScripts\(\w+\.\w+/, "'/'+location.pathname.split('/').slice(0, -4).join('/')") + const newContent = content.replace(/importScripts\((['"])([^'"]+)(['"])\)/, + "importScripts($1" + "/'+location.pathname.split('/').slice(0, -4).join('/')+'/$3)") fs.writeFileSync(filePath, newContent, 'utf8') - patched = true + patched = matches.length > 0 } if (!patched) throw new Error('No importScripts found in any worker files') }
🧹 Nitpick comments (10)
renderer/rsbuildSharedConfig.ts (1)
64-72: Good implementation for worker handling in single file builds.This change correctly sets up the worker-rspack-loader to handle
.worker.jsand.worker.tsfiles when building in single file mode. Theinline: "no-fallback"option ensures these workers are properly bundled inline, which aligns with the PR objective of moving Minecraft protocol handling to a separate worker thread.Consider adding a brief comment explaining why this specific loader configuration was chosen and what "no-fallback" implies for browser compatibility, to help future maintainers understand the design decisions.
src/protocolWorker/protocol.worker.ts (3)
14-18: Optional: Consider a uniform approach for event redirection
There's a TODO comment to eventually send "all events" rather than enumerating them. One approach is to automatically proxy every emitted event from the Minecraft protocol instead of a hardcoded list.
78-85: Heads-up aboutskipWriteLogusage
Overriding theclient.writemethod for packet logging, gated behindskipWriteLog, may obscure important debugging data ifskipWriteLogremains true longer than intended. Consider resetting it automatically or offering a manual toggle to avoid losing valuable logs.
175-223: Add handling for invalid PEM strings
Currently, ifrestoreDataencounters an invalid or corrupted PEM, it won’t throw. Consider a try/catch aroundimportKeyto handle failures gracefully and notify the main thread or user of invalid keys.src/index.ts (3)
180-181: Validate parsed server address
IfconnectOptions.serveris malformed or empty,parseServerAddressmight fail. Add checks to handle invalid results to prevent runtime crashes.
194-194: Caution with detailed logs
PrintingserverParsed.serverIpFullis helpful for debugging, but consider omitting sensitive or internal addresses in production logs for privacy or security compliance.
522-522: Remove commented code
// lastPacket = fullBuffer.toString()is commented out. If no longer needed, removing it would improve clarity and maintainability.src/protocolWorker/protocolMain.ts (3)
19-30: Consider built-in clone or structuredClone
copyPrimitiveValuesselectively clones some properties. If a full deep clone is needed, usingstructuredCloneor a library might reduce boilerplate and handle more data types.
52-62: Ensure coordinated Worker teardown
Terminating the worker via asetTimeoutcallback tied tobot.on('end')or the absence ofbotcan become tricky if multiple bots are created. Evaluate a more robust teardown to avoid worker leaks.
162-179: Performance overhead of JSON serialization
Forwarding method calls throughJSON.stringify/parseis valid, but repeated large data calls can degrade performance. Consider more direct message passing or Transferable objects if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
renderer/rsbuildSharedConfig.ts(1 hunks)rsbuild.config.ts(3 hunks)src/index.ts(9 hunks)src/protocolWorker/protocol.worker.ts(1 hunks)src/protocolWorker/protocolMain.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/protocolWorker/protocol.worker.ts (3)
renderer/viewer/lib/workerProxy.ts (1)
createWorkerProxy(1-19)src/mineflayer/websocket-core.ts (1)
getWebsocketStream(17-54)src/mineflayer/minecraft-protocol-extra.ts (1)
validatePacket(57-78)
🪛 GitHub Actions: CI
rsbuild.config.ts
[error] 207-207: ENOENT: no such file or directory, scandir './dist/single'
src/protocolWorker/protocolMain.ts
[error] 1-1: Build error: Failed to resolve loader: worker-rspack-loader in /home/runner/work/minecraft-web-client/minecraft-web-client, error: NotFound("worker-rspack-loader")
🔇 Additional comments (7)
src/protocolWorker/protocol.worker.ts (1)
137-173:❓ Verification inconclusive
Confirm compatibility of Web Crypto in worker
TheprotocolMicrosoftAuth.authenticateflow andrestoreDatarely onwindow.crypto. Double-check that your Worker environment supports these Web Crypto APIs and that sensitive key material isn’t improperly exposed.
🏁 Script executed:
#!/bin/bash # Checking references to 'crypto' usage in worker code rg -A 3 'window[.]crypto[.]'Length of output: 621
Ensure Worker Compatibility for Web Crypto APIs
The authentication flow (via
protocolMicrosoftAuth.authenticateandrestoreData) depends on Web Crypto. We found that the worker code insrc/protocolWorker/protocol.worker.tsinvokes Web Crypto usingwindow.crypto.subtlefor key import/export. In a traditional Web Worker, however, the global object is typicallyself(orglobalThis), notwindow. Please verify that your target Worker environment provides the expected Web Crypto APIs under the given reference. If not, consider updating the code to useself.cryptoorglobalThis.cryptoto avoid potential runtime issues and ensure that sensitive key material remains secure.src/index.ts (4)
76-76: Smooth usage of reactive updates
Importingsubscribefrom 'valtio' fits well with the rest of the reactive state management. This looks good.
98-98: Centralizing protocol logic
ImportinggetProtocolClientGetteris a neat way to offload protocol tasks to the Worker. This promotes a cleaner architecture by separating connection details from the main thread.
189-190: Check proxy address validity
Similar to server parsing, ensureconnectOptions.proxyis valid. If it's malformed or not set,proxyParsed.hostorproxyParsed.portmay be undefined, causing connection errors.
452-457: Clarify conditional returns inget client()
When certain conditions are met, the getter returnsundefinedinstead of a protocol client. Potentially document or handle the scenario to avoid confusion in code expecting a client instance.src/protocolWorker/protocolMain.ts (2)
13-14: Strong typing for protocol worker
DefiningprotocolWorkerChannelwith a type referencingPROXY_WORKER_TYPEis a good way to maintain type safety when communicating via messages.
193-197: Conditional authentication flow
InvokingauthFlowMainThreadonly ifauthDatais present keeps the worker flexible for both online and offline scenarios. Good approach for modular and optional Microsoft auth.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
rsbuild.config.ts (1)
266-282: 🛠️ Refactor suggestionThe worker import path patching implementation has improved but still has potential issues
The implementation addresses some of the concerns from previous reviews but still has potential issues:
- The regex replacement logic in line 274-275 is still brittle and may not handle all import path formats.
- The function doesn't handle the case where there are no worker files at all.
- The patched flag logic is improved but the error message doesn't provide enough context.
Consider implementing a more robust solution:
const patchWorkerImport = () => { if (!fs.existsSync('./dist/static/js/async')) { console.warn('No async directory found, skipping worker import patching') return } const workerFiles = fs.readdirSync('./dist/static/js/async').filter(x => x.endsWith('.js')) if (workerFiles.length === 0) { console.warn('No worker files found, skipping worker import patching') return } let patched = false for (const file of workerFiles) { const filePath = `./dist/static/js/async/${file}` const content = fs.readFileSync(filePath, 'utf8') const matches = content.match(/importScripts\([^)]+\)/g) || [] if (matches.length > 1) throw new Error('Multiple importScripts found in ' + filePath) - const newContent = content.replace(/importScripts\(\w+\.\w+/, - "importScripts('/'+location.pathname.split('/').slice(0, -4).join('/')") + const newContent = content.replace(/importScripts\((['"])([^'"]+)(['"])\)/, + "importScripts($1" + "/'+location.pathname.split('/').slice(0, -4).join('/')+'/$3)") if (newContent !== content) { fs.writeFileSync(filePath, newContent, 'utf8') patched = true } } if (!patched) throw new Error('No importScripts patched in any worker files. This might indicate a change in the worker code structure.') }Additionally, consider adding debug logging to help with troubleshooting:
console.log(`Patching ${workerFiles.length} worker files in ./dist/static/js/async`)
🧹 Nitpick comments (1)
rsbuild.config.ts (1)
212-217: Add error handling for empty async directory checkGood addition of directory existence check before attempting to read it. This prevents build failures when the directory doesn't exist and aligns with a previous review comment.
However, the current implementation could be more robust with better error messages.
Consider providing more context in the error message to help developers understand the issue:
if (fs.existsSync('./dist/static/js/async')) { const asyncFiles = fs.readdirSync('./dist/static/js/async') if (asyncFiles.length > 0) { - throw new Error('dist/static/js/async must be empty. Ensure workers are imported & built correctly.') + throw new Error('dist/static/js/async must be empty for single file builds. Found files: ' + asyncFiles.join(', ')) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rsbuild.config.ts(3 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
rsbuild.config.ts
[error] 207-207: Failed to build. ENOENT: no such file or directory, scandir './dist/single'
🔇 Additional comments (1)
rsbuild.config.ts (1)
234-235: Worker import patching added before service worker generationGood placement of the
patchWorkerImport()function call before the service worker generation to ensure worker imports are correctly patched.
|
/protocol |
|
/deploy |
|
Deployed to Vercel Preview: https://prismarine-r0lk6nejb-zaro.vercel.app |
PR Type
+10% perf on high load servers (1k packets from server) like hypixel: packet serialization / deserialization now happens in its own thread
Enhancement, Bug fix
Description
Introduced a new
protocolWorkermodule for handling Minecraft protocol tasks.protocolMain.tsandprotocolWorker.tsfor managing protocol-related operations.Refactored authentication flow to streamline Microsoft account integration.
microsoftAuthflowlogic toprotocolWorker/microsoftAuthflow.authFlowMainThreadandgetAuthDatafor improved token management.Improved version handling and utility functions.
toMajorVersionutility to a more appropriate location.Removed unused or redundant code for better maintainability.
Changes walkthrough 📝
9 files
Added `toMajorVersion` utility functionUpdated imports to include `toMajorVersion`Refactored connection logic and removed unused codeRefactored Microsoft authentication flowAdded main protocol handling logicIntroduced worker for protocol-related tasksUpdated imports for `toMajorVersion`Removed `toMajorVersion` and cleaned up utilitiesUpdated imports for refactored authentication flow1 files
Simplified packet validation logic1 files
Added global configurations for protocol workerSummary by CodeRabbit
New Features
Refactor