Conversation
thefeymesaleng
changed the title
|
Hey @thefeymesaleng |
commit: |
|
Hey @thefeymesaleng, Can you please add some tests to |
There was a problem hiding this comment.
Pull request overview
This PR adds support for localDependencies so users can pass local functions into workers (addressing the historical isoworker/localDeps limitations) and wires this feature into the examples app.
Changes:
- Extend
useWorkeroptions and worker code generation to acceptlocalDependencies, inject their definitions into the worker script, and pass them alongside existingremoteDependencies. - Update
remoteDepsParserandcreateWorkerBlobUrlto build worker blob code that includes both remote script imports and serialized local functions. - Add a new
LocalDepsexample page and route to demonstrate usinglocalDependenciesin the examples app.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/useWorker/src/useWorker.ts |
Extends the options type and default options with localDependencies, passes them to createWorkerBlobUrl, and preserves existing worker lifecycle behavior. |
packages/useWorker/src/lib/remoteDepsParser.ts |
Generalizes the parser to handle both remote URLs and local dependency functions by emitting importScripts(...) and function definitions into the worker script. |
packages/useWorker/src/lib/createWorkerBlobUrl.ts |
Updates the worker blob code generator to accept localDeps, delegate to the new remoteDepsParser signature, and keeps the rest of the worker bootstrap unchanged. |
apps/examples/src/pages/LocalDeps/index.jsx |
Introduces a new example demonstrating useWorker with localDependencies using a simple pow helper and UI to trigger and observe the worker. |
apps/examples/src/App.jsx |
Registers the new LocalDeps demo in the navigation and router so the example can be accessed from the examples app. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .map((fn) => { | ||
| const str = fn.toString(); | ||
| if (str.trim().startsWith("function")) { | ||
| return str; | ||
| } else { | ||
| const name = fn.name; |
There was a problem hiding this comment.
When building the depsFunctionString, this code assumes that every localDeps entry is a named function and blindly emits const ${name} = ${str} for non-function declarations. If a consumer passes an anonymous arrow/function expression (e.g. localDependencies: [() => 1]), fn.name will be an empty string and the generated worker script will contain invalid syntax like const = () => 1, causing the worker creation to fail with a syntax error. Consider either enforcing named functions (and throwing/logging when fn.name is empty) or generating a safe unique identifier when fn.name is falsy, so the generated worker code is always valid JavaScript.
| .map((fn) => { | |
| const str = fn.toString(); | |
| if (str.trim().startsWith("function")) { | |
| return str; | |
| } else { | |
| const name = fn.name; | |
| .map((fn, index) => { | |
| const str = fn.toString(); | |
| if (str.trim().startsWith("function")) { | |
| return str; | |
| } else { | |
| const name = | |
| fn.name && fn.name.trim().length > 0 ? fn.name : `_localDep_${index}`; |
| const createWorkerBlobUrl = ( | ||
| fn: Function, | ||
| deps: string[], | ||
| transferable: TRANSFERABLE_TYPE /* localDeps: () => unknown[], */, | ||
| localDeps: Function[], | ||
| transferable: TRANSFERABLE_TYPE /* localDeps: () => unknown[], */ |
There was a problem hiding this comment.
The createWorkerBlobUrl signature now includes a localDeps parameter, but the JSDoc above still only documents fn and deps, which can confuse consumers and future maintainers about how to pass local dependencies. Please update the JSDoc to include the localDeps parameter (its expected type and behavior) so the documentation stays consistent with the function signature and the new localDependencies option exposed via useWorker.
I noticed that this repository does not currently support
localDependenciesdue to challenges with the isoworker implementation. Last year, I submitted a PR to vueuse for useWebWorkerFn( a feature that is based on this repo) to addlocalDependencies. To align with vueuse's preference for a dependency-free core package, I avoided using isoworker, and use a simpler implementation. While we already solve the localDeps problems, I think this implementation should work the same for this repo. So I hope @alewin can review this PR and your user can enjoy localDeps. Cheers.This PR should address the following issues: #112, #105.
PS
Sorry for the formatting