Skip to content
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 transpiling some builds to ES5 #1435

Merged
merged 4 commits into from
Jun 17, 2024

Conversation

peaBerberian
Copy link
Collaborator

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 peaBerberian changed the title Stop transpiling some builds to ES5 [Proposal] Stop transpiling some builds to ES5 Apr 30, 2024
@peaBerberian peaBerberian force-pushed the misc/stop-transpiling-to-es5 branch from dc76c23 to e5b98a1 Compare April 30, 2024 16:01
depending on your project).
If you need to provide support for the `MULTI_THREAD` feature on those platforms, we
recommend that you use a transpiler tool on that worker file to make it compatible to ES5.
Examples of transpiler tools are [babel](https://babeljs.io/) and [swc](https://swc.rs/).
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pro-tip: we can just throw the hot potato to the application :D

package.json Outdated Show resolved Hide resolved
scripts/run_bundler.mjs Show resolved Hide resolved
@peaBerberian peaBerberian force-pushed the misc/stop-transpiling-to-es5 branch from e5b98a1 to 8d29944 Compare May 2, 2024 14:16
@peaBerberian peaBerberian added the Priority: 0 (Very high) This issue or PR has a very high priority. Efforts should be concentrated on it first. label Jun 10, 2024
@peaBerberian peaBerberian added this to the 4.1.0 milestone Jun 10, 2024
@peaBerberian peaBerberian mentioned this pull request Jun 13, 2024
@peaBerberian peaBerberian force-pushed the misc/stop-transpiling-to-es5 branch from 8d29944 to 7d62d36 Compare June 13, 2024 15:31
For purposes where we need to generate a bundle (the demo, the worker
files, the RxPlayer bundle builds), we're progressively removing our
reliance on webpack toward esbuild.

It began with the demo page originally for performance reasons, but we
found out that the esbuild behavior and API was much simpler to
understand than webpack's, and so we ended up also prefering to rely on
it for our worker build.

Considering that we since the v4 only rely on TypeScript with no
bundling step for our "regular" build (and our home-made scripts in the
`./scripts` directory), we thus ended up having no reliance on webpack
for most of our builds.

However at #1400, we noticed that we might also need to provide an ES5
version of our worker file (at least for the PlayStation 4), and I found
that having ES5 support through esbuild is not straightforward
(evanw/esbuild#297).

Historically, [babeljs](https://babeljs.io/) was used to translate to
ES5 - and that's the one I know, so I tried using it on the bundle made
by esbuild.
However it turns out that there's no resource for how to use babel
like this - not integrated as a bundler's plugin (here we just wanted to
translate an ES2017 already-made bundle to es5) so I just gave up and
relied on webpack for specifically that ES5 build.

However this both added some perceptible delay in our build, and we
now relied on two different bundlers in our `npm run build` main
build script, which made me un-comfortable in terms of maintainance and
understanding of how our build step worked.

So I re-looked around and read about [`swc`](https://swc.rs/), which
seems to be some kind of babel competitor with some credibility (though
they advertises themselves as a """platform""", I would have preferred a
clearer term but ok), with some differences, including speed, but what is
most interesting here is that it can just be run as a standalone
transpiler (what I mean: not embedded in a bundler).

By using it, I've been able to remove reliance on webpack (and babel) in
our regular build steps (produced by `npm run build`) which should be
realistically used by 99.9999% of applications (approximately!). The
only exception where both webpack and babel are still used is to produce
our bundles attached to each release notes (they expose the RxPlayer
through `window`, like our grandfathers did) - which I guess are rarely
relied upon.
Still, we should probably remove reliance on webpack there in the
future.

Swc is maintained apparently by volunteers, and is less known than
babeljs, so there's still that. Also, the bundler and transpiler
JavaScript landscape is __VERY__ active (even in JavaScript terms) right
now with projects like [rolldown](https://rolldown.rs/) and
[oxc](https://oxc-project.github.io/) so sadly it may be not the last
update of this part of our build process.
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.
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 peaBerberian force-pushed the misc/stop-transpiling-to-es5 branch from 7d62d36 to 4cf92c7 Compare June 17, 2024 16:31
@peaBerberian peaBerberian merged commit 2b8dcfd into dev Jun 17, 2024
6 checks passed
@peaBerberian peaBerberian deleted the misc/stop-transpiling-to-es5 branch July 26, 2024 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: 0 (Very high) This issue or PR has a very high priority. Efforts should be concentrated on it first.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants