Skip to content
This repository has been archived by the owner on Sep 9, 2021. It is now read-only.

Issue with publicPath in v3 #281

Open
ksocha opened this issue Aug 12, 2020 · 15 comments
Open

Issue with publicPath in v3 #281

ksocha opened this issue Aug 12, 2020 · 15 comments

Comments

@ksocha
Copy link

ksocha commented Aug 12, 2020

  • 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
 
@alexander-akait
Copy link
Member

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

@alexander-akait
Copy link
Member

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

@ksocha
Copy link
Author

ksocha commented Aug 12, 2020

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?

@alexander-akait
Copy link
Member

alexander-akait commented Aug 12, 2020

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
Copy link
Member

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

@clinyong
Copy link

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.

@alexander-akait
Copy link
Member

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

@clinyong
Copy link

@Tianruo
Copy link

Tianruo commented Aug 15, 2020

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

@alexander-akait
Copy link
Member

@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
Copy link
Member

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
Copy link
Member

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

@alexander-akait
Copy link
Member

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

@mwanago
Copy link

mwanago commented Nov 24, 2020

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

@alexander-akait
Copy link
Member

You can find PR above your comment

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants