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

Find an alternative to Esprima #8056

Open
tykus160 opened this issue Feb 11, 2025 · 6 comments
Open

Find an alternative to Esprima #8056

tykus160 opened this issue Feb 11, 2025 · 6 comments
Assignees
Labels
component: build system The issue involves the build system of Shaka Player type: code health A code health issue type: enhancement New feature or request
Milestone

Comments

@tykus160
Copy link
Member

Have you read the FAQ and checked for duplicate open issues?
Y

Is your feature request related to a problem? Please describe.

Esprima, the tool we're using during externs generation, is no longer supported. Due to that we are limited in terms of progressing with the codebase, i.e. we are not able to enable more modern JS syntax.

Describe the solution you'd like

Check can we use any of modern alternatives. Most natural seems to be espree which is based on Esprima and provides similar API.

Describe alternatives you've considered

Additional context

Are you planning to send a PR to add it?

@tykus160 tykus160 added component: build system The issue involves the build system of Shaka Player type: code health A code health issue type: enhancement New feature or request labels Feb 11, 2025
@tykus160 tykus160 added this to the v4.14 milestone Feb 11, 2025
@tykus160
Copy link
Member Author

It seems attachComment option is not supported anymore. Instead, there's a comment flag which adds comments array at the top of parsed object (not included in body anymore). It needs to be taken into account.

@joeyparrish
Copy link
Member

I think the most sensible thing to do is to make a concrete plan for TypeScript conversion. We won't need esprima after that.

@tykus160
Copy link
Member Author

@joeyparrish totally agree, simple check shows that esprima -> espree is not a trivial transition. We need to make a plan for Typescript (& new build system?) conversion though, otherwise tech debt will become larger.

@joeyparrish
Copy link
Member

Yes, I agree. I want to make a concrete plan, and soon. I will experiment with it to see if I can find a transition plan that allows for something other than "all or nothing" conversion, since it will take some time to rewrite everything.

As for the build system, I'm open to suggestions. I have a little experience with TypeScript with eslint & webpack now, but I am not strongly opinionated about the bundler.

@tykus160
Copy link
Member Author

I'm not super oriented in bundlers right now, but instead of webpack I'd consider rspack due to much better performance observed.

@absidue
Copy link

absidue commented Feb 15, 2025

I know you probably haven't reached the planning stage just yet, but I still hope it is okay to provide a few ideas and opinions on the migration :).

As shaka-player's code base consists mainly of classes and the Closure Compiler is as far as I know the only bundler and mimifier that is able to properly mimify and treeshake classes, the bundle size will likely go up. It's not the end of the world though, as there are solutions to those problems in the modern JavaScript world.

Any class that currently only contains static methods and properties, could be converted to just be a bunch of functions and variables. If you still want them grouped together when you import them into other files, you can just use the namespace object syntax import * as someName from './someFile.js' that way you get to keep using them with a prefix but when the bundler looks at it, it'll be able to transform it into direct function references (and potentially even inline the functions).

While it might be tempting to use TypeScript's namespaces to recreate the current namespace structure, I would advise against it, as they just get transformed into objects at build time (aka more bloat). Even the TypeScript compiler has switched away from using TypeScript namespaces in their codebase.

Please prefer top level exports instead of exporting everything inside one object like you currently do e.g. import { Player } from 'shaka-player', if people still want to have the shaka-player prefix they can just do import * as shaka from 'shaka-player'. Most projects these days use some kind of bundler instead of importing the entire library from a CDN, so being able to get the code as small as possible and remove unused bits of code, is highly desired. If you still want to provide a CDN friendly bundle you can still easily do that by creating a simple file like this and running a bundler on it:

import * as shaka from 'shaka-player'

window.shaka = shaka

It would also be great if shaka-player were more treeshaking friendly, as currently the only was to reduce the size of the bundle is to create a custom build. One obvious example is shaka-player's plugin system, which could be exposed to the library users so they only need to register what they need and the unused plugins (e.g. text parsers) could be treeshaken away. For users that want an easy way to start and don't care about the bundle size too much you could still make the default entry point (exports field in the package.json file), register everything.
Example of what something like that could look like from a users perspective:

import { registerTextParser, VttTextParser } from 'shaka-player/modular'

registerTextParser('text/vtt', VttTextParser)

Alternate example that makes it easier to register a text parser for multiple mime type in one go, by providing arrays of standard mimetypes:

import { registerTextParser, VttTextMimeTypes, VttTextParser } from 'shaka-player/modular'

registerTextParser(VttTextMimeTypes, VttTextParser)

// internally VttTextMimeTypes could be written as this:
export const VttTextMimeTypes = ['text/vtt', 'text/vtt; codecs="vtt"','text/vtt; codecs="wvtt"']

These are all just ideas and personal opinions, from someone that welcomes the modernisation of the code base and would love to see it being used as an oportunity to make shaka-player smaller without having to create custom builds, so please do not feel obliged to do any of these things

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: build system The issue involves the build system of Shaka Player type: code health A code health issue type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants