-
Notifications
You must be signed in to change notification settings - Fork 133
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
Proposal: Stop relying on webpack for legacy bundles generation #1425
Closed
peaBerberian
wants to merge
1
commit into
misc/remove-webpack-from-worker-bundling
from
misc/remove-webpack-from-legacy-bundles
Closed
Proposal: Stop relying on webpack for legacy bundles generation #1425
peaBerberian
wants to merge
1
commit into
misc/remove-webpack-from-worker-bundling
from
misc/remove-webpack-from-legacy-bundles
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As a follow-up effort to #1420 (on which this work is based on), I attempt here to stop relying on webpack for the generation of our "legacy bundles", which are rarely used RxPlayer bundles that are communicated through release notes and which exposes the `RxPlayer` class through the global scope (`window`). The idea, the same than for #1420, was to simplify our bundling process by relying only on esbuild for this (instead of both webpack and esbuild depending on the situation). I chose esbuild over webpack here mainly because I better understand its role, behavior and most-of-all its API than webpack's - though its (much) faster bundling speed is a also a very good argument for this. Like for #1420, the last remaining issue was our usage of the babeljs transpiler to transpile to ES5 (our legacy bundles are ES5), but that's been fixed by switching to swc for that part. Our legacy bundle was purely configured by the `webpack.config.js` file at the root of our project that I here removed. To replace it, I re-purposed our `scripts/bundle_worker.mjs` script into a more generic `scripts/run_bundler.mjs` script for which we're now supposed to indicate the wanted input and output files. -- Still, I encountered an important issue while doing that in that I did not understand how we're supposed to indicate to esbuild that our entry file's export has to be exported through the global scope (all my introduction about the esbuild API being simpler to understand now loses all credibility!). I tried playing with its [`globalName`](https://esbuild.github.io/api/#global-name) and [`format`](https://esbuild.github.io/api/#format) configuration options which seem linked to that but couldn't make it work. For now, I gave up, adding a `__GLOBAL_SCOPE__` compile-time constant (boolean) to `src/index.ts` which has to be set to `true` before bundling if you want to export the RxPlayer through the global scope.
peaBerberian
force-pushed
the
misc/remove-webpack-from-legacy-bundles
branch
2 times, most recently
from
April 9, 2024 15:32
41b409a
to
3f6a3ff
Compare
NOTE: I found out that I didn't at all understand the |
peaBerberian
added
work-in-progress
This Pull Request or issue is not finished yet
Priority: 3 (Low)
This issue or PR has a low priority.
proposal
This Pull Request or Issue is only a proposal for a change with the expectation of a debate on it
labels
Apr 10, 2024
peaBerberian
added a commit
that referenced
this pull request
Apr 30, 2024
This is built on top of #1425 itself built on top of #1420, which removes most dependencies to webpack (besides tests, that we'll need to also change anyway considering the fact that `karma-webpack` has an old reported issue on macOS with our build). We planned to release in our future 4.1.0 an ES5 version of our worker file and as a lesser feature an ES5 "legacy" build (the one linked to release notes - not the one published on npm). Providing an ES5 version of our worker file was especially added to support the `MULTI_THREAD` feature on PlayStation 4 devices. However I sadly propose here that we roll back that attempt of support (just of the `MULTI_THREAD` feature, the PlayStation 4 stay officially supported and tested). --- After multiple attempts (myself and then @Florent-Bouisset), we realized the complexities and costs of maintaining a supplementary ES5 version of builds. We initially tried to switch from a webpack+babel mix to swc on a bundle already produced by esbuild. The global idea was to simplify our bundling process by designing an architecturally simple and low-maintainance process made of a separate bundling step then transpiling step on the bundling result. But we realized that this philosophy doesn't seem to be compatible to those tools. When combined with the fact that their documentation is often either lacking or too complex, than some of our questionning to maintainers led to poor answers and that the JS tooling ecosystem appears to be still changing very fast, I'm not sure that I want to guarantee support of ES5 for our hopefully long-lived v4 right now.
peaBerberian
added a commit
that referenced
this pull request
Apr 30, 2024
This is built on top of #1425 itself built on top of #1420, which removes most dependencies to webpack (besides tests, that we'll need to also change anyway considering the fact that `karma-webpack` has an old reported issue on macOS with our build). We planned to release in our future 4.1.0 an ES5 version of our worker file and as a lesser feature an ES5 "legacy" build (the one linked to release notes - not the one published on npm). Providing an ES5 version of our worker file was especially added to support the `MULTI_THREAD` feature on PlayStation 4 devices. However I sadly propose here that we roll back that attempt of support (just of the `MULTI_THREAD` feature, the PlayStation 4 stay officially supported and tested). --- After multiple attempts (myself and then @Florent-Bouisset), we realized the complexities and costs of maintaining a supplementary ES5 version of builds. We initially tried to switch from a webpack+babel mix to swc on a bundle already produced by esbuild. The global idea was to simplify our bundling process by designing an architecturally simple and low-maintainance process made of a separate bundling step then transpiling step on the bundling result. But we realized that this philosophy doesn't seem to be compatible to those tools. When combined with the fact that their documentation is often either lacking or too complex, than some of our questionning to maintainers led to poor answers and that the JS tooling ecosystem appears to be still changing very fast, I'm not sure that I want to guarantee support of ES5 for our hopefully long-lived v4 right now.
peaBerberian
added a commit
that referenced
this pull request
Apr 30, 2024
This is built on top of #1425 itself built on top of #1420, which removes most dependencies to webpack (besides tests, that we'll need to also change anyway considering the fact that `karma-webpack` has an old reported issue on macOS with our build). We planned to release in our future 4.1.0 an ES5 version of our worker file and as a lesser feature an ES5 "legacy" build (the one linked to release notes - not the one published on npm). Providing an ES5 version of our worker file was especially added to support the `MULTI_THREAD` feature on PlayStation 4 devices. However I sadly propose here that we roll back that attempt of support (just of the `MULTI_THREAD` feature, the PlayStation 4 stay officially supported and tested). --- After multiple attempts (myself and then @Florent-Bouisset), we realized the complexities and costs of maintaining a supplementary ES5 version of builds. We initially tried to switch from a webpack+babel mix to swc on a bundle already produced by esbuild. The global idea was to simplify our bundling process by designing an architecturally simple and low-maintainance process made of a separate bundling step then transpiling step on the bundling result. But we realized that this philosophy doesn't seem to be compatible to those tools. When combined with the fact that their documentation is often either lacking or too complex, than some of our questionning to maintainers led to poor answers and that the JS tooling ecosystem appears to be still changing very fast, I'm not sure that I want to guarantee support of ES5 for our hopefully long-lived v4 right now.
peaBerberian
added a commit
that referenced
this pull request
May 2, 2024
This is built on top of #1425 itself built on top of #1420, which removes most dependencies to webpack (besides tests, that we'll need to also change anyway considering the fact that `karma-webpack` has an old reported issue on macOS with our build). We planned to release in our future 4.1.0 an ES5 version of our worker file and as a lesser feature an ES5 "legacy" build (the one linked to release notes - not the one published on npm). Providing an ES5 version of our worker file was especially added to support the `MULTI_THREAD` feature on PlayStation 4 devices. However I sadly propose here that we roll back that attempt of support (just of the `MULTI_THREAD` feature, the PlayStation 4 stay officially supported and tested). --- After multiple attempts (myself and then @Florent-Bouisset), we realized the complexities and costs of maintaining a supplementary ES5 version of builds. We initially tried to switch from a webpack+babel mix to swc on a bundle already produced by esbuild. The global idea was to simplify our bundling process by designing an architecturally simple and low-maintainance process made of a separate bundling step then transpiling step on the bundling result. But we realized that this philosophy doesn't seem to be compatible to those tools. When combined with the fact that their documentation is often either lacking or too complex, than some of our questionning to maintainers led to poor answers and that the JS tooling ecosystem appears to be still changing very fast, I'm not sure that I want to guarantee support of ES5 for our hopefully long-lived v4 right now.
For now closed in profit of #1435 |
Merged
peaBerberian
added a commit
that referenced
this pull request
Jun 13, 2024
This is built on top of #1425 itself built on top of #1420, which removes most dependencies to webpack (besides tests, that we'll need to also change anyway considering the fact that `karma-webpack` has an old reported issue on macOS with our build). We planned to release in our future 4.1.0 an ES5 version of our worker file and as a lesser feature an ES5 "legacy" build (the one linked to release notes - not the one published on npm). Providing an ES5 version of our worker file was especially added to support the `MULTI_THREAD` feature on PlayStation 4 devices. However I sadly propose here that we roll back that attempt of support (just of the `MULTI_THREAD` feature, the PlayStation 4 stay officially supported and tested). --- After multiple attempts (myself and then @Florent-Bouisset), we realized the complexities and costs of maintaining a supplementary ES5 version of builds. We initially tried to switch from a webpack+babel mix to swc on a bundle already produced by esbuild. The global idea was to simplify our bundling process by designing an architecturally simple and low-maintainance process made of a separate bundling step then transpiling step on the bundling result. But we realized that this philosophy doesn't seem to be compatible to those tools. When combined with the fact that their documentation is often either lacking or too complex, than some of our questionning to maintainers led to poor answers and that the JS tooling ecosystem appears to be still changing very fast, I'm not sure that I want to guarantee support of ES5 for our hopefully long-lived v4 right now.
peaBerberian
added a commit
that referenced
this pull request
Jun 17, 2024
This is built on top of #1425 itself built on top of #1420, which removes most dependencies to webpack (besides tests, that we'll need to also change anyway considering the fact that `karma-webpack` has an old reported issue on macOS with our build). We planned to release in our future 4.1.0 an ES5 version of our worker file and as a lesser feature an ES5 "legacy" build (the one linked to release notes - not the one published on npm). Providing an ES5 version of our worker file was especially added to support the `MULTI_THREAD` feature on PlayStation 4 devices. However I sadly propose here that we roll back that attempt of support (just of the `MULTI_THREAD` feature, the PlayStation 4 stay officially supported and tested). --- After multiple attempts (myself and then @Florent-Bouisset), we realized the complexities and costs of maintaining a supplementary ES5 version of builds. We initially tried to switch from a webpack+babel mix to swc on a bundle already produced by esbuild. The global idea was to simplify our bundling process by designing an architecturally simple and low-maintainance process made of a separate bundling step then transpiling step on the bundling result. But we realized that this philosophy doesn't seem to be compatible to those tools. When combined with the fact that their documentation is often either lacking or too complex, than some of our questionning to maintainers led to poor answers and that the JS tooling ecosystem appears to be still changing very fast, I'm not sure that I want to guarantee support of ES5 for our hopefully long-lived v4 right now.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Priority: 3 (Low)
This issue or PR has a low priority.
proposal
This Pull Request or Issue is only a proposal for a change with the expectation of a debate on it
work-in-progress
This Pull Request or issue is not finished yet
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As a follow-up effort to #1420 (on which this work is based on), I attempt here to stop relying on webpack for the generation of our "legacy bundles", which are rarely used RxPlayer bundles that are communicated through release notes and which exposes the
RxPlayer
class through the global scope (window
).The idea, the same than for #1420, was to simplify our bundling process by relying only on esbuild for this (instead of both webpack and esbuild depending on the situation). I chose esbuild over webpack here mainly because I better understand its role, behavior and most-of-all its API than webpack's - though its (much) faster bundling speed is a also a very good argument for this.
Like for #1420, the last remaining issue was our usage of the babeljs transpiler to transpile to ES5 (our legacy bundles are ES5), but that's been fixed by switching to swc for that part.
Our legacy bundle was purely configured by the
webpack.config.js
file at the root of our project that I here removed.To replace it, I re-purposed our
scripts/bundle_worker.mjs
script into a more genericscripts/run_bundler.mjs
script for which we're now supposed to indicate the wanted input and output files.--
Still, I encountered an important issue while doing that in that I did not understand how we're supposed to indicate to esbuild that our entry file's export has to be exported through the global scope (all my introduction about the esbuild API being simpler to understand now loses all credibility!). I tried playing with its
globalName
andformat
configuration options which seem linked to that but couldn't make it work.For now, I gave up, adding a
__GLOBAL_SCOPE__
build-time constant (boolean) tosrc/index.ts
which has to be set totrue
before bundling if you want to export the RxPlayer through the global scope.