Conversation
DO NOT MERGE This is incomplete but just looking at some of the changes required to provide worker as a separate file - only for kmw so far. Build-bot: skip Test-bot: skip
User Test ResultsTest specification and instructions User tests are not required Test Artifacts
|
|
@jahorton FYI this is what I was playing with in terms of unbundling the worker; I have not yet tackled the path questions (need some special casing for unit tests, but for normal use it should be easier I hope, relative to keymanweb.js init path). And of course the Android bypass of importScript is really a separate question altogether. Are you keen to take this on? Or would you prefer that I keep working on this? |
* Rename several classes and files to clarify difference between worker and factory that constructs worker * Rework worker factories to actually implement `WorkerFactory` interface * Fixup pathing issues where `http:/host` was returned instead of `http://host` * Rename worker-thread output from worker-main.js to worker-thread.js, resolving confusion with separate worker-main module * Add path resolution for worker-thread.js to Keyman Engine * Tweak build scripts and unit tests to work with the new unbundled paths
| SRC_ROOT="${KEYMAN_ROOT}/web/src/app/browser/src" | ||
|
|
||
| node_es_bundle "${SRC_ROOT}/debug-main.js" \ | ||
| node_es_bundle "${SRC_ROOT}/main.js" \ |
There was a problem hiding this comment.
Looks like we should delete debug-main.js and release-main.js (like we did in webview).
| /** | ||
| * Defines mappings from Node Worker signatures to WebWorker signatures | ||
| * | ||
| * TODO: move this to a separate module, no need for it to be embedded string |
There was a problem hiding this comment.
Might be good to do this before merging this PR - would make it probably easier to understand
|
|
||
| const __filename__ = fileURLToPath(import.meta.url); | ||
| const __dirname__ = path.dirname(__filename__); | ||
| const workerThreadFilename = __dirname__ + "/../../../../worker-thread/build/lib/worker-thread.js"; |
There was a problem hiding this comment.
Is this always the right path? in browser/build.sh and webview/build.sh we copy it to ${BUILD_ROOT}/release/worker-thread.js.
| ${scriptStr} | ||
| `; | ||
| const buffer = Buffer.from(concatenatedScript); | ||
| export class NodePredictiveTextWorker extends worker.Worker implements Worker { |
There was a problem hiding this comment.
NodePredictiveTextWorkerFactory mentions that this is only used in unit tests. Would be good to add that here as well.
There was a problem hiding this comment.
Filename should probably be node-predictive-text-worker-factory.ts to be consistent
There was a problem hiding this comment.
Filename should probably be web-predictive-text-worker-factory.ts
2 participants
DO NOT MERGE
This is incomplete but just looking at some of the changes required to provide worker as a separate file - only for kmw so far.
WorkerFactoryinterfacehttp:/hostwas returned instead ofhttp://hostBuild-bot: release
Test-bot: skip