chore: handle esm output for recent module bundler
Why
I did not find in pull request history or issues the subject of exporting some remotion libs to ES modules. As node now handle them natively we could consider using it when we drop Node10 support. This PR keep both CommonJs and Esmodule output.
As the attribute "type":"module" is not set, Node won't use natively this new output. For that we would need to delete the cjs output. This is only usefull now for module bundlers.
That's why I thought this could be a good step to ESM compatibility/migration
- esbuild documentation on the subject
- Tutorial to publish both ESM and CJS
- Awesome article from @sindresorhus about ESM
How
- define a
esmtypescrypt output - use it in public package
- handle
build:esmpipeline with turbo distfolder now have 2 subfolders with CommonJS and Esmodulespackage.jsonexport main, types and module attributes
This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.
remotion – ./
🔍 Inspect: https://vercel.com/remotion/remotion/F8nZgXPKN36bMcEfeokcduyZdgbg
✅ Preview: https://remotion-git-fork-slashgear-esm-output-remotion.vercel.app
Thanks a lot for tackling this! This is great and will prepare us for the future.
I will only merge it with much more scrutiny than usual though, since I've seen many libraries in the JS ecosystem publish with ES Module support only to find out it broke something. Once you think it's ready and all the pipelines pass, I'll probably make a prerelease and test it in all kind of scenarios.
I think i fixed some issues, i am in 🏝 and cab’t work on it until end of month
@Slashgear Enjoy your vaction!
I tryed some solution for my test errors but I am a bit stuck now. You may have an idea @JonnyBurger ? 💡
We need to replace all require()'s with fs.readFileSync or imports. I pushed a few refactors - let's see if it removes a few errors.
I am having a few doubts whether we want to ship this anytime soon when we get it working. Pondering....:
- We need to remain CJS support for those who want to use SSR APIs and are not yet ready to migrate to ESM
- As long as we remain CJS support, we cannot use the new features (like
import.meta.url) because it only works in ESM - Most people cannot move to ESM because they have 1000 dependencies and if one of them doesn't support ESM, stuff breaks
- Nobody has ever asked me for a ESM version of Remotion
- We lock ourselves out of using a dependency that does not support ESM in the future
I'm sorry when I am so pessimistic when ESM is the future and I appreciate all your work to get it supported! Since if we ship it, it will only be opt-in, I don't see a big harm either. It will also make it easier to support other runtimes like Deno or Bun and help us modernize. But we need to do some testing and should it be too much of a pain for us, or users, then I'm not ruling out holding off ESM support indefinitely.
I'm going to revisit this PR soon since now this feature is more demanded than ever and now I understand it better myself!
I have taken a small part of this PR and will implement ESM for the @remotion/player (#1745) now since that is the main package that gets imported in users web apps and it had an ESM problem (see #1709).
I'll let it play out for a few versions and if everything goes well, then I'll come back to this PR and implement ESM for more packages.
Revisiting my comments from last year...
We lock ourselves out of using a dependency that does not support ESM in the future
I think this is not true, we can import modules which at least support "faux ESM"
We need to remain CJS support for those who want to use SSR APIs and are not yet ready to migrate to ESM
This is true, I think we have to accept the fact that we need to ship two versions
Most people cannot move to ESM because
People are now actively using Vite, making it quite popular
So, I was kind of wrong :) Sorry about that!
The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Comments | Updated |
|---|---|---|---|---|
| remotion | ❌ Failed (Inspect) | Jan 25, 2023 at 2:56PM (UTC) |
Socket Security Pull Request Report
Dependency issues detected. If you merge this pull request, you will not be alerted to the instances of these issues again.
😵💫 Bin script confusion
This package has multiple bin scripts with the same name. This can cause non-deterministic behavior when installing or could be a sign of a supply chain attack
Consider removing one of the conflicting packages. Packages should only export bin scripts with their name
| Package | Bin script | Source |
|---|---|---|
| [email protected] (added) | jest |
packages/cli/package.json |
| [email protected] (added) | jest |
packages/cli/package.json via [email protected] |
Pull request report summary
| Issue | Status |
|---|---|
| Install scripts | ✅ 0 issues |
| Native code | ✅ 0 issues |
| Bin script confusion | ⚠️ 2 issues |
| Bin script shell injection | ✅ 0 issues |
| Unresolved require | ✅ 0 issues |
| Invalid package.json | ✅ 0 issues |
| HTTP dependency | ✅ 0 issues |
| Git dependency | ✅ 0 issues |
| Potential typo squat | ✅ 0 issues |
| Known Malware | ✅ 0 issues |
| Telemetry | ✅ 0 issues |
| Protestware/Troll package | ✅ 0 issues |
Bot Commands
To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore [email protected] [email protected]
@SocketSecurity ignore [email protected]@SocketSecurity ignore [email protected]
Powered by socket.dev
We now support ESM for most modules which get bundled in the browser, such as remotion, @remotion/player, @remotion/gif etc.!
More things were necessary to satisfy the entirety of the JS ecosystem, I'll make a video about the entire process! Thanks for spearheading this and being aware of ESM way before we were!