worker-loader icon indicating copy to clipboard operation
worker-loader copied to clipboard

Issue with publicPath in v3

Open ksocha opened this issue 3 years ago • 15 comments

  • Operating System: MacOS
  • Node Version: 12.16.1
  • NPM Version: 6.13.4
  • webpack Version: 4.44.1
  • worker-loader Version: 3.0.1

Expected Behavior

I would expect that setting publicPath in options would take precedence over __webpack_public_path__ variable. We use webpacker and setting a CDN path to serve our assets. However, worker must be served from same domain in order to work.

I had to use patch-package to revert to one of previous versions and fit it my needs (code below).

Actual Behavior

With our setup, passing publicPath does nothing.

Code

patch-package snippet

diff --git a/node_modules/worker-loader/dist/utils.js b/node_modules/worker-loader/dist/utils.js
index abe26e2..89089ce 100644
--- a/node_modules/worker-loader/dist/utils.js
+++ b/node_modules/worker-loader/dist/utils.js
@@ -62,12 +62,16 @@ function workerGenerator(loaderContext, workerFilename, workerSource, options) {
 
   const esModule = typeof options.esModule !== 'undefined' ? options.esModule : true;
 
+  const publicPath = typeof options.publicPath === 'undefined'	
+      ? '__webpack_public_path__'	
+      : JSON.stringify(options.publicPath);
+
   if (options.inline) {
     const InlineWorkerPath = (0, _loaderUtils.stringifyRequest)(loaderContext, `!!${require.resolve('./runtime/inline.js')}`);
     let fallbackWorkerPath;
 
     if (options.inline === 'fallback') {
-      fallbackWorkerPath = `__webpack_public_path__ + ${JSON.stringify(workerFilename)}`;
+      fallbackWorkerPath = `${publicPath} + ${JSON.stringify(workerFilename)}`;
     }
 
     return `
@@ -76,7 +80,7 @@ ${esModule ? `import worker from ${InlineWorkerPath};` : `var worker = require($
 ${esModule ? 'export default' : 'module.exports ='} function() {\n  return worker(${JSON.stringify(workerSource)}, ${JSON.stringify(workerConstructor)}, ${JSON.stringify(workerOptions)}, ${fallbackWorkerPath});\n}\n`;
   }
 
-  return `${esModule ? 'export default' : 'module.exports ='} function() {\n  return new ${workerConstructor}(__webpack_public_path__ + ${JSON.stringify(workerFilename)}${workerOptions ? `, ${JSON.stringify(workerOptions)}` : ''});\n}\n`;
+  return `${esModule ? 'export default' : 'module.exports ='} function() {\n  return new ${workerConstructor}(${publicPath} + ${JSON.stringify(workerFilename)}${workerOptions ? `, ${JSON.stringify(workerOptions)}` : ''});\n}\n`;
 } // Matches only the last occurrence of sourceMappingURL
 

ksocha avatar Aug 12 '20 11:08 ksocha

You need rewrite __webpack_public_path__ in this case on fly, also you can use publicPath as Function

alexander-akait avatar Aug 12 '20 11:08 alexander-akait

Previous implementation was wrong and doesn't work with async chunks and global publicPath option

alexander-akait avatar Aug 12 '20 11:08 alexander-akait

Using publicPath as Function makes no difference - it's still not working for my use case. Rewriting __webpack_public_path__ is also not an option, as it would probably break other loaders?

ksocha avatar Aug 12 '20 11:08 ksocha

Rewriting webpack_public_path is also not an option, as it would probably break other loaders?

It is only working solution in your case, __webpack_public_path__ is primary value for files from public path, if you need to override it, please do it on fly, just rewrite it on value that you need and return old value

alexander-akait avatar Aug 12 '20 11:08 alexander-akait

Also you do not fill out the issue template and without reproducible test repo or steps, I can't say you the best solution

alexander-akait avatar Aug 12 '20 11:08 alexander-akait

Previous implementation was wrong and doesn't work with async chunks and global publicPath option

@evilebottnawi Should the Cross-Origin Policy section update for v3. I meet the Cross-Origin Policy problem, and set the publicPath option as the readme said

module.exports = {
  module: {
    rules: [
      {
        loader: 'worker-loader',
        options: { publicPath: '/workers/' },
      },
    ],
  },
};

But it is not work for v3, which (the same publicPath setting) work for v2.

clinyong avatar Aug 14 '20 10:08 clinyong

@clinyong Can you provide reproducible test repo? it is tested and works fine

alexander-akait avatar Aug 14 '20 11:08 alexander-akait

the publicPath option does't work at all, it always use out.publicPath to require my js, if it's the original purpose, I have to go back to v2

Tianruo avatar Aug 15 '20 09:08 Tianruo

@clinyong I think here we have misleading docs, in fact we have two publicPath here:

  1. publicPath for non worker chunks (including async chunks) - main.js (output.publicPath)
  2. publicPath for worker chunks (including async chunks) - echo.worker.worker.js (worker-loader.options.publicPath)

Why? Because there are 2 cases:

  • You want to put worker initial chunk (echo.worker.worker.js) and worker async chunks (if you have import something from 'something' in worker code) in separate directory, here you need to use worker-loader.options.publicPath.
  • You want to override publicPath for place where you load worker, using publicPath is not help here, because webpack by default uses __webpack_public_path__.

alexander-akait avatar Aug 17 '20 15:08 alexander-akait

In this case you need override public path on fly:

index.js

import './rewritePublicPath';
import EchoWorker from "./echo.worker";

const echoWorker = new EchoWorker();

echoWorker.onmessage = function (e) {
  console.log("Message received from worker");
  console.log(e.data);
};

setTimeout(() => {
  console.log("Post message to worker");
  echoWorker.postMessage([1, 2]);
}, 1000);

rewritePublicPath.js

// You can use `Define` plugin to avoid hardcodding value to code
__webpack_public_path__ = "http://127.0.0.1:5000/dist/";

alexander-akait avatar Aug 17 '20 15:08 alexander-akait

But maybe we can introduce loadedPublicPath option to override it inside loader

alexander-akait avatar Aug 17 '20 16:08 alexander-akait

I think we can improve inline option for this case and using code from here https://benohead.com/blog/2017/12/06/cross-domain-cross-browser-web-workers/, no CORS issues, no extra content in bundle

alexander-akait avatar Aug 20 '20 17:08 alexander-akait

@evilebottnawi This would be awesome. Any news on the fix for this?

mwanago avatar Nov 24 '20 14:11 mwanago

You can find PR above your comment

alexander-akait avatar Nov 24 '20 14:11 alexander-akait