-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Add ability to inject functions into the workers #1235
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1235 +/- ##
==========================================
+ Coverage 85.86% 88.61% +2.75%
==========================================
Files 77 78 +1
Lines 4307 4384 +77
==========================================
+ Hits 3698 3885 +187
+ Misses 609 499 -110
Continue to review full report at Codecov.
|
Most plugins are asset types and already run in workers right? |
src/workerfarm/WorkerFarm.js
Outdated
this.localWorker = require('./childBootstrap'); | ||
this.registerWorkerFunctions(this.options.workerPath || '../worker', [ | ||
'run', | ||
'init' |
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.
Maybe we could autodetect these instead of needing to list them? Basically Object.keys(require(workerPath))
?
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.
I can change it, but think it'll be more precise if it has to be defined when being called.
For example if a worker has more than just the worker functions as exports we don't add a handler for all of them.
If we document this eventually it should be fine to use export keys I guess.
@devongovett Yes that's true, the typescript one utilises workers for speeding up error handling. It would be faster and probably better if the plugin wouldn't have to start and handle it's own workers. |
Also seems like this would potentially lead to different modules overwriting each other if they had the same function names. What if instead we just made the |
@devongovett If this doesn't impact performance or code readability it's probably a great idea. Would definitely solve the problem of 2 plugins possibly having a naming conflict |
Changes the way workers are used from registering functions/file to just requesting a function with it's path. Using mkHandle
Example: