Skip to content
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

electron - bundling with asar gives 'Cannot find module 0.bundle.worker.js' error #17

Closed
dinataranis opened this issue Mar 15, 2020 · 33 comments · Fixed by andywer/threads.js#226

Comments

@dinataranis
Copy link

Hi!

I am building an electron application with electron-webpack and electron-builder.
I am trying to use threadsjs.
When testing in dev mode everything works great, but unfortunately when trying to pack the app (using electron-builder), with the asar option, I am getting the following error:
`Cannot find module 0.bundle.worker.js``

I think it is caused by a mismatch in the worker's path.
Since packing with asar option causes the entire app to be bundled as a single file, it can't access the worker at runtime.

After searching the web for a bit I found the asarUnpack option (which specifies which files to unpack when creating the asar archive) is probably what we are looking for, but I think it requires changes in threads-plugin to allow worker files to be bundle as asarUnpack.
Code example with asarUnpack flag:

"asarUnpack": [
  "dist/main/0.bundle.worker.js",
  "dist/main/0.bundle.worker.js.map"
]

I have reproduced the issue with the basic template, it can be accessed from here.
When bundling with asar: false everything works as expected, but without we get the error above.

Bundling with asar: false is strongly not recommended solution, and it really affects the app performance.

We will be thankful if this can be solved.

P.S. I took the dependencies from my current project, but I get the same error, when I use the latest

Best regards.

@andywer
Copy link
Owner

andywer commented Mar 15, 2020

Hey @dinataranis, thanks for reporting.

How do you think would the solution look like? The threads-plugin cannot set the asarUnpack option as that latter one is in electron-builder.

I could imagine that setting the asarUnpack option manually (could also set it to something generic like dist/main/*.worker.js*, I think) would make the app work. Have you tried yet?

@dinataranis
Copy link
Author

Hi, @andywer, sorry for delay.
I tried to set asarUnpack manually as you advised (it saves *.worker.js* files in app.asar.unpacked instead of packing them in app.asar), but in the release mode I still get the same error Cannot find module ...resources\app.asar\dist\main\0.bundle.worker.js
If I understand it correctly, threads-plugin changes the paths given anywhere I use new Worker('./relative/path/to/worker)
This causes 'threads' to search for my worker in app.asar file. but files cannot be accessed externally within app.asar, so we have to unpack it as you suggested. nevertheless, we still need to notify threads to search in the new path (a.k.a app.asar.unpacked/dist/main/0.bundle.worker.js)
For that I think we need your help

@andywer
Copy link
Owner

andywer commented Mar 18, 2020

I am not yet 100% sure how the solution eventually needs to look like, but I think we need some way to optionally pass a custom worker-path-rewrite function to the threads-plugin, so you can use something like this.

@raz-sinay
Copy link

raz-sinay commented Mar 18, 2020

Hi @andywer,
I took a look into this problem as well, and I think what you've suggested is the right way to go.
something like an options object passed to

new Worker(path, {
    overrideResolvedPath: (resolvedPath:string)=>string;
})

(That's in typescript to make it more clear.)

Just for checking, we have changed the code under implementation.node.js

function resolveScriptPath(scriptPath) {
    // eval() hack is also webpack-related
    const workerFilePath = typeof __non_webpack_require__ === "function"
        ? __non_webpack_require__.resolve(path.join(eval("__dirname"), scriptPath))
        : require.resolve(rebaseScriptPath(scriptPath, /[\/\\]worker_threads[\/\\]/));
    // ~~~~~~~~hardcoded check~~~~~~~~~~~
    return workerFilePath.replace('app.asar', 'app.asar.unpack');
}

And it does find the module, but unfortunately, we get another error:

Cannot find module 'source-map-support/source-map-support.js'

It makes sense since in 0.bundle.worker.js starts like this:

require("source-map-support/source-map-support.js").install(),function(e){var t={};function r(n){if(t[n])return t[n].exports;var o=t[n]={i:n,l:!1,exports:{}};return e[n].call(o.exports,o,o.exports,r),o.l=!0,o.exports}r.m=e,r.c=t,r.d=function(e,t,n){r.o(e,t)||Object.defineProperty(e,t,{enumerable:!0,get:n})},r.r=function(e){"undefined"!=typeof Symbol&&Symbol.toStringTag&&Object.defineProperty(e,Symbol.toStringTag,{value:"Module"}),Object.defineProperty(e,"__esModule",{value:!0})},r.t=function(e,t){if(1&t&&
.... more code

I'm not sure if it's related to threads or threads-plugin though.

Any ideas?

@andywer
Copy link
Owner

andywer commented Mar 18, 2020

@raz-sinay No, the option should not go into the Worker instantiation, but be passed to the threads-plugin instantiation in the webpack config, so it can rewrite the path at build time.

Might find some time to look into that tonight.

@andywer
Copy link
Owner

andywer commented Mar 20, 2020

@raz-sinay @dinataranis Can you try to apply this little patch to your node_modules/threads-plugin?

Don't forget to update the dist/threads-plugin.js and set the rewritePath option on new ThreadsPlugin() in the webpack config 😉

diff --git a/src/index.js b/src/index.js
index f944e7c..e5515e8 100644
--- a/src/index.js
+++ b/src/index.js
@@ -31,6 +31,7 @@ export default class WorkerPlugin {
 
   apply (compiler) {
     let workerId = 0;
+    const rewritePath = this.options.rewritePath || (workerPath => workerPath);
 
     compiler.hooks.normalModuleFactory.tap(NAME, factory => {
       for (const type of JS_TYPES) {
@@ -74,7 +75,8 @@ export default class WorkerPlugin {
             }
 
             const loaderOptions = { name: opts.name || workerId + '' };
-            const req = `require(${JSON.stringify(workerLoader + '?' + JSON.stringify(loaderOptions) + '!' + dep.string)})`;
+            const workerPath = rewritePath(dep.string);
+            const req = `require(${JSON.stringify(workerLoader + '?' + JSON.stringify(loaderOptions) + '!' + workerPath)})`;
             const id = `__webpack__worker__${workerId++}`;
             ParserHelpers.toConstantDependency(parser, id)(expr.arguments[0]);

@dinataranis
Copy link
Author

Hi, @andywer
Thanks for your update.
I tried your solution, but I got the same error.
As I see in dist/threads-plugin.js we apply the rewritePath function to the worker's relative path instead of absolute

const workerPath = rewritePath('./worker.js');

@andywer
Copy link
Owner

andywer commented Mar 22, 2020

@dinataranis @raz-sinay Ohh, sh**, you guys are right, of course. Sorry for the confusion…

@dinataranis Can you try monkey-patching implementation.node.js in threads to replace app.asar with app.asar.unpack as Raz did?

@raz-sinay The source-map-support issue seems to be unrelated to threads/threads-plugin. Maybe an npm ls source-map-support explains where it comes from?

@dinataranis
Copy link
Author

Hi, @andywer
I changed the code in implementation.node.js to replace app.asar with app.asar.unpack and got another error:

Cannot find module 'source-map-support/source-map-support.js.

So I added source-map-support and all it's dependencies to the asarUnpack option in package.json and ... it works!

"asarUnpack": [
  "dist/main/*.worker.js*",
  "node_modules/source-map/**/*",
  "node_modules/source-map-support/**/*",
  "node_modules/buffer-from/**/*",
]

It means, that all node modules which I use in workers files I have to include to asarUnpack option.
It will be great if you can fix the workerFilePath path in theradsjs/thereads-plugin modules.
Thank you for you help!

@andywer
Copy link
Owner

andywer commented Mar 25, 2020

@dinataranis Can you try npm install threads@1.3.1-asar.unpack? It should work without any extra options besides the asarUnpack in the package.json. The node.js workers will automatically try the *.asar.unpack path if the resolved worker path contains .asar/ and instantiation failed.

@andywer
Copy link
Owner

andywer commented Mar 31, 2020

Ping 👉 @dinataranis.

@dinataranis
Copy link
Author

Hi, @andywer
I have tried the threads@1.3.1-asar.unpack version, and I got the same error Cannot find module '...\resources\app.asar\dist\main\0.bundle.worker.js'
You can see it in my updated project here
I debugged it a little and in the release mode in the file threads\dist-esm\master\implementation.node.js in Worker constructor if I print the final path string throw resolvedScriptPath I still see the ...resources\app.asar\dist\main\0.bundle.worker.js

constructor(scriptPath, options) {
    const resolvedScriptPath = resolveScriptPath(scriptPath);
    if (resolvedScriptPath.match(/\.tsx?$/i) && detectTsNode()) {
        super(createTsNodeModule(resolvedScriptPath), { eval: true });
    }
    else if (resolvedScriptPath.match(/\.asar\//)) {
        try {
            super(resolvedScriptPath, options);
        }
        catch (_a) {
            // See <https://github.com/andywer/threads-plugin/issues/17>
            super(resolvedScriptPath.replace(/\.asar([/\/])/, ".asar.unpack$1"), options);
        }
    }
    else {
        super(resolvedScriptPath, options);
    }

    throw resolvedScriptPath;
    
    this.mappedEventListeners = new WeakMap();
    allWorkers.push(this);
}

@dinataranis
Copy link
Author

dinataranis commented Mar 31, 2020

The small update.
I think that the problem could be in this condition resolvedScriptPath.match(/\.asar\//) in implementation.node.js file.
I have reproduced it in js console and got null

var path = "resources\app.asar\dist\main\0.bundle.worker.js"
path.match(/\.asar\//) 
// null

It is because of the backslashes in the string. If we try to escape them first and only after what make the search, it should work.

var path = "resources\\app.asar\\dist\\main\\0.bundle.worker.js"
path.match(/\.asar[\\,\/]/)

@andywer
Copy link
Owner

andywer commented Mar 31, 2020

Yeah, you can monkey-patch it for now using .match(/\.asar[\\\/]/). In the .replace() call the regex is correct (it's wrong in a different way, actually), it's wrong in the condition only (don't ask me why…).

@andywer
Copy link
Owner

andywer commented Mar 31, 2020

Try this commit 😉
andywer/threads.js@ab2f3b9

@andywer
Copy link
Owner

andywer commented Mar 31, 2020

I published the latest feature branch commit using a testing tag. Let me know if this one works for you.

npm install threads@1.3.1-asar.unpack.2

@dinataranis
Copy link
Author

Hi, @andywer
Just a few remarks about 1.3.1-asar.unpack.2 version.
When I have installed it

"dependencies": {
    "threads": "1.3.1-asar.unpack.2",
    ...
}

I didn't get your fresh changes, but I just added them manually (changed the match condition to /\.asar[\/\\]/, replace condition to (/\.asar([\/\\])/ and instead of .asar.unpack$1 should be .asar.unpacked$1) and it works as expected!!

@andywer
Copy link
Owner

andywer commented Apr 1, 2020

@dinataranis Ohhh. That was me being stupid while publishing. Can you give 1.3.1-asar.unpack.3 a try? That one the latest feature branch commit for real.

@dinataranis
Copy link
Author

Just a tiny thing, you have forgotten to change .asar.unpack$1 to .asar.unpacked$1

@raz-sinay
Copy link

raz-sinay commented Apr 5, 2020

@andywer Was that merged to some version we can use?

@andywer
Copy link
Owner

andywer commented Apr 5, 2020

Ohh man. I was so busy, I completely forgot. It's merged and published as threads.js v1.4.0 now! 🙌

@raz-sinay
Copy link

raz-sinay commented Apr 5, 2020

I think something is wrong with the regex. because I'm still getting:
Cannot find module 'path\to\resources\app.asar\dist\main\1.bundle.worker.js'

@andywer
Copy link
Owner

andywer commented Apr 6, 2020

@raz-sinay To be on the safe side, can you list the following information?

  • Your node.js version
  • The version in your node_modules/threads/package.json
  • Is tiny-worker installed?
  • You are using Windows, right?

@raz-sinay
Copy link

raz-sinay commented Apr 6, 2020

Sure,

  • NodeJS version is integrated within electron . So, electron version is 7.1.14 which contains NodeJS 12.8.1
  • node_modules/threads/package.json - 1.4.0,
  • tiny-worker isn't installed and wasn't installed before also.
  • I am using windows.

When I test the path against the regex /\.asar[\/\\]/ It seems that it doesn't match because the path is given with a single \. I mean: C:\path\to\worker
when I escape it to C:\\path\\to\\worker the regex above does find a match. I'm not sure that's the issue, but it might help investigating so i'm mentioning it.

@andywer
Copy link
Owner

andywer commented Apr 6, 2020

Not sure if it's the regex. I just quickly tried it in a REPL and it matched:

> "C:\\path\\to\\resources\\app.asar\\dist\\main\\1.bundle.worker.js".match(/\.asar[\/\\]/)
[".asar\", index: 24, input: "C:\path\to\resources\app.asar\dist\main\1.bundle.worker.js", groups: undefined]

Did you set the asarUnpack property in the package.json?

@raz-sinay
Copy link

I did, also made sure that it was unpacked.
Did it work for you even with a single \ ? I mean unescaped path?

@raz-sinay
Copy link

raz-sinay commented Apr 6, 2020

@andywer We did some more investigation.
found out that:

// node_modules\threads\dist-esm\master\implementation.node.js
 class Worker extends NativeWorker {
        constructor(scriptPath, options) {
            const resolvedScriptPath = resolveScriptPath(scriptPath, (options || {})._baseURL);
            if (resolvedScriptPath.match(/\.tsx?$/i) && detectTsNode()) {
                super(createTsNodeModule(resolvedScriptPath), Object.assign(Object.assign({}, options), { eval: true }));
            }
            else if (resolvedScriptPath.match(/\.asar[\/\\]/)) {
                try {             
                    **** // If I add here throw new Error(), everything works as expected ****
                    super(resolvedScriptPath, options);  // This line doesn't throw error                    
                }
                catch (_a) {
                    // See <https://github.com/andywer/threads-plugin/issues/17>
                    super(resolvedScriptPath.replace(/\.asar([\/\\])/, ".asar.unpacked$1"), options);
                }
            }
            else {
                super(resolvedScriptPath, options);
            }
            this.mappedEventListeners = new WeakMap();
            allWorkers.push(this);
        }

@raz-sinay
Copy link

@andywer any news with this one?

@andywer
Copy link
Owner

andywer commented Apr 19, 2020

@raz-sinay Ohh, sorry.

Did it work for you even with a single \ ? I mean unescaped path?

Don't have a Windows system to test, besides in CI.

I guess we should not catch and then fix the path, but just assume we need to fix it or maybe add an option to control the ASAR path rewriting, but let it default to rewrite.

@andywer andywer reopened this Apr 19, 2020
@raz-sinay
Copy link

raz-sinay commented Apr 19, 2020

I guess we should not catch and then fix the path, but just assume we need to fix it or maybe add an option to control the ASAR path rewriting, but let it default to rewrite.

Sounds good.

@andywer
Copy link
Owner

andywer commented Apr 19, 2020

I opened a follow-up PR andywer/threads.js#236 and published it under a testing tag as threads@1.4.0-resolve-asar-immediately.

@raz-sinay Give it a try – hope that does the job 😉

@raz-sinay
Copy link

I opened a follow-up PR andywer/threads.js#236 and published it under a testing tag as threads@1.4.0-resolve-asar-immediately.

@raz-sinay Give it a try – hope that does the job

Works!

@andywer andywer closed this as completed May 1, 2020
@jonluca
Copy link

jonluca commented Apr 21, 2023

This should probably be reverted (or optional) now that electron natively supports asar's electron/electron@06a00b7

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 a pull request may close this issue.

4 participants