-
-
Notifications
You must be signed in to change notification settings - Fork 273
feat: add publicPath
support (options.publicPath
)
#31
feat: add publicPath
support (options.publicPath
)
#31
Conversation
options.proxy
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs Rebase && Tests
.gitignore
Outdated
node_modules | ||
.idea |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert this
Can you rebase please ? So I can finally review this ? :) |
ee1a2c4
to
4993636
Compare
@michael-ciniawsky: rebased and added a unit test now that unit tests are supported. |
4993636
to
7e58614
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe better to name the option publicPath
instead ?
cc @TrySound
index.js
Outdated
@@ -9,7 +9,8 @@ const SingleEntryPlugin = require('webpack/lib/SingleEntryPlugin'); | |||
const schema = require('./options.json'); | |||
|
|||
const getWorker = (file, content, options) => { | |||
const workerPublicPath = `__webpack_public_path__ + ${JSON.stringify(file)}`; | |||
const basePath = options.proxy ? JSON.stringify(options.proxy) : '__webpack_public_path__'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
basePath
=> publicPath
or inline it into the template literal
options.proxy
)publicPath
support (options.publicPath
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DullReferenceException Could you just add |
or fix them all good 😛 |
54c0d89
to
7dc6d39
Compare
publicPath
support (options.publicPath
)publicPath
support (options.publicPath
)
@DullReferenceException Please one last rebase sry 😛 @TrySound @d3viant0ne @evilebottnawi Any objections against this ? Please review when time 🙃 |
README.md
Outdated
@@ -151,3 +170,8 @@ const worker: Worker = new MyWorker(); | |||
|
|||
[test]: http://img.shields.io/travis/webpack-contrib/worker-loader.svg | |||
[test-url]: https://travis-ci.org/webpack-contrib/worker-loader | |||
|
|||
|
|||
## License |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be here but should go away on the last rebase
README.md
Outdated
@@ -91,6 +91,25 @@ self.postMessage({foo: 'foo'}) | |||
self.addEventListener('message', (event) => { console.log(event); }); | |||
``` | |||
|
|||
|
|||
## Cross-origin policy workarounds | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we describe the feature to be less use case specific?
Feel free to add the worker cross-origin policy as an example underneath.
- Add
publicPath
to table that describes the api - Add
## Public Path
docs below the existing sections describing the api options - Add generic example
- Add note about workaround for cross-origin specific usecase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@michael-ciniawsky Other than the documentation, it looks fine pending a thumbs up from @TrySound
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 I've rebased & updated the documents.
7dc6d39
to
3a481c3
Compare
Added the ability to use a different public path when loading workers as a way to work around cross-origin policy issues.
3a481c3
to
84f555d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our web app hosts static assets on a CDN, making the loading of web worker scripts difficult due to cross-origin policies. The inline option works for most browsers, but we started experiencing some difficulties with Safari due to its handling of Unicode in blobs being inconsistent with other browsers. This proxy option lets the worker load from a same-origin URL; in our case, we have some middleware in which we proxy the request to the webpack dev server (for local development) or serve up a local file.
Noteable Changes
Added the ability to use a proxy URL when loading workers as a way to work around cross-origin policy issues.