-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Improved Documentation #7682
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
base: main
Are you sure you want to change the base?
Improved Documentation #7682
Conversation
@pranjalisr is attempting to deploy a commit to the OpenJS Foundation Team on Vercel. A member of the Team first needs to authorize it. |
src/content/guides/web-workers.mdx
Outdated
/* global __webpack_public_path__, __webpack_require__ */ | ||
|
||
// Ensure the worker knows where to load chunks from | ||
__webpack_public_path__ = __webpack_require__.p = self.location.origin + '/assets/'; |
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.
the documentation around public path is correct, but as a user you shouldnt tap into the generated bundle/file directly.
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.
the documentation around public path is correct, but as a user you shouldnt tap into the generated bundle/file directly.
Thanks for pointing that out. I’ve made some changes that don’t go directly into the generated bundle/file. Please have a look and share your feedback.
src/content/guides/web-workers.mdx
Outdated
--- | ||
Note that this is only available in ESM. `Worker` in CommonJS syntax is not supported by either webpack or Node.js. | ||
## Advanced: Cross-Domain and Dynamic Web Workers |
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.
No need to prefix "Advanced"
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 have removed the prefix "Advanced". Do you suggest anymore changes?
### Setting a dynamic public path safely | ||
The recommended approach is to define `__webpack_public_path__` at the beginning of your worker entry file, before any imports. |
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.
where did you find the documentation for this?
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.
where did you find the documentation for this?
The note about defining webpack_public_path before any imports is based on the official https://webpack.js.org/guides/public-path/#on-the-fly documentation.
Summary
Added Cross-Domain and Dynamic Web Workers to fix issue 6716
What kind of change does this PR introduce?
Added Cross-Domain and Dynamic Web Workers
Did you add tests for your changes?
Not Required
Does this PR introduce a breaking change?
No
If relevant, what needs to be documented once your changes are merged or what have you already documented?