search

webpack-contrib / worker-loader

A webpack loader that registers a script as a Web Worker
MIT License
1.46k stars 273 forks source link

feat: add option `crossOrigin` #291

Open pckhoi opened 4 years ago

pckhoi commented 4 years ago

This PR contains a:

Motivation / Use-Case

Solving https://github.com/webpack-contrib/worker-loader/issues/281 according to this comment

Breaking Changes

No breaking changes.

Additional Info

codecov[bot] commented 4 years ago

Codecov Report

Merging #291 (3fcc7a2) into master (2448e13) will decrease coverage by 6.17%. The diff coverage is 30.43%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #291      +/-   ##
==========================================
- Coverage   76.35%   70.17%   -6.18%     
==========================================
  Files           6        7       +1     
  Lines         148      171      +23     
  Branches       52       62      +10     
==========================================
+ Hits          113      120       +7     
- Misses         30       44      +14     
- Partials        5        7       +2
Impacted Files Coverage Δ
src/runtime/crossOrigin.js 0.00% <0.00%> (ø)
src/index.js 95.34% <100.00%> (+0.11%) :arrow_up:
src/utils.js 88.37% <100.00%> (+1.88%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2448e13...3fcc7a2. Read the comment docs.

pckhoi commented 4 years ago

@evilebottnawi how is my solution? If you think it is good then I'll try to improve code coverage.

mwanago commented 4 years ago

Hello. Any plans on going forward with this one? 💔

pckhoi commented 4 years ago

Could use my fork for now?

mwanago commented 4 years ago

@pckhoi I don't seem to be able to use a relative path here.

options: {
  crossOrigin: '/static-proxy/'
}

Uncaught DOMException: Failed to execute 'importScripts' on 'WorkerGlobalScope': The URL '/static-proxy/worker.aeb45db0.worker.js' is invalid.

Assuming my page is http://reddit.com, I would like to add /static-proxy/ as my originPath. This would result in using http://reddit.com/static-proxy/${fileName} as the path to the worker.

Can this be achieved? Unfortunately, the URL of my page is dynamic, and therefore the originPath has to be relative.

alexander-akait commented 4 years ago

@mwanago Yep, we need thinking more about this and solve this in more general way

mwanago commented 4 years ago

@evilebottnawi Maybe allowing a function to be used as the crossOrigin (or another name we come up with) would help. I could use ${window.location.origin}/static-proxy as the return of the function.

Instead of 

options: {
  crossOrigin: '/static-proxy/'
}

do

options: {
  crossOrigin: () => (
    `${window.location.origin}/static-proxy/`
  )
}
alexander-akait commented 4 years ago

We should use currentScript here, webpack supports publicPath: 'auto'...

mwanago commented 4 years ago

@evilebottnawi My public path is set to something else and I need it to stay this way. I need to override it only for the worker path.

alexander-akait commented 4 years ago

@mwanago Just for information, can you provide real use case?

pckhoi commented 4 years ago

@mwanago this PR is meant to solve cross origin use cases. I think you might be able to do what you want with existing options already. What have you tried so far? Would changing __webpack_public_path__ before importing worker script not work?

Also just noticed we also have a publicPath option. This is confusing. Does it only override Webpack's publicPath?

mwanago commented 4 years ago

@pckhoi This is precisely due to me wanting to solve my cross-origin issue.

@evilebottnawi I've created a small proxy on my Node.js backend that is in the same origin and under the hood loads content from the CDN. Now I want to use this proxy to load my worker.

@pckhoi Changing the __webpack_public_path__ before importing the worker script didn't work well for me because I wasn't able to change it back to the previous value. Trying to change it back caused it to work like I never changed __webpack_public_path__ before importing.

alexander-akait commented 4 years ago

sounds resonable, I will thinking about this deeply in near future, right now I am focused on webpack-dev-server updating, so ping me at this Friday and we try to solve this

mwanago commented 4 years ago

ping me at this Friday and we try to solve this

@alexander-akait I am pinging you 👀

mwanago commented 4 years ago

ping me at this Friday and we try to solve this

@alexander-akait Is there any hope for tackling this issue?

alexander-akait commented 4 years ago

Sorry for delay, deadline + a lot of issues, I remember about this, try to return to this in near future, sorry again

mwanago commented 3 years ago

Sorry for delay, deadline + a lot of issues, I remember about this, try to return to this in near future, sorry again

Is there any way I can help you with that?

alexander-akait commented 3 years ago

As minimum we need better name for this option

mwanago commented 3 years ago

@alexander-akait I think workerChunkUrl sounds fine, as long as we allow it to be a relative path.