remotion icon indicating copy to clipboard operation
remotion copied to clipboard

chore: handle esm output for recent module bundler

Open Slashgear opened this issue 3 years ago • 6 comments

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

How

  • define a esm typescrypt output
  • use it in public package
  • handle build:esm pipeline with turbo
  • dist folder now have 2 subfolders with CommonJS and Esmodules
  • package.json export main, types and module attributes

Slashgear avatar Mar 15 '22 21:03 Slashgear

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

vercel[bot] avatar Mar 15 '22 21:03 vercel[bot]

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.

JonnyBurger avatar Mar 16 '22 09:03 JonnyBurger

I think i fixed some issues, i am in 🏝 and cab’t work on it until end of month

Slashgear avatar Mar 18 '22 16:03 Slashgear

@Slashgear Enjoy your vaction!

JonnyBurger avatar Mar 23 '22 12:03 JonnyBurger

I tryed some solution for my test errors but I am a bit stuck now. You may have an idea @JonnyBurger ? 💡

Slashgear avatar Mar 26 '22 22:03 Slashgear

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.

JonnyBurger avatar Mar 29 '22 09:03 JonnyBurger

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!

JonnyBurger avatar Jan 25 '23 14:01 JonnyBurger

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)

vercel[bot] avatar Jan 25 '23 14:01 vercel[bot]

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]

Powered by socket.dev

socket-security[bot] avatar Jan 25 '23 14:01 socket-security[bot]

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!

JonnyBurger avatar Mar 16 '23 10:03 JonnyBurger