-
-
Notifications
You must be signed in to change notification settings - Fork 851
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
Added type=module to @ffmpeg/util package.json #627
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for ffmpegwasm canceled.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a change to the build step might be needed here since the current setup is compiling both ESM and CJS code to the .js
extension. Specifically line 11 and 12 of the exports will be problematic with the project's type set to module
.
I personally use the convention of explicit .cjs
/ .mjs
extensions when both CommonJS and ESM are in play, which helps make it clearer what one can expect from each file's behavior. It would also cut down on bundle size in this case, since the ./esm and ./cjs subfolders can be eliminated.
@jeromewu have you considered using tsup or vite to build your project? They can make things a lot simpler compared to just working with the typescript compiler.
Thanks!
@@ -3,6 +3,7 @@ | |||
"version": "0.12.1", | |||
"description": "browser utils for @ffmpeg/*", | |||
"main": "./dist/cjs/index.js", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With type set to module
, these file extensions will either need to be .cjs
or have a package.json file created in their respective ./cjs folder that has its type set to commonjs
. Otherwise the module resolution API will treat them all as ESM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is quiet hard to find a perfect solution for building and bundling, will test and see if tsup is better in this scenario if I have time. Thanks!
Just a little +1 from me. I'm seeing the same error when using Remix. |
I ran into the same problem. If you need a work around, this is what worked for me.
npm install patch-package --save -dev
// package.json
"scripts": {
...
"postinstall": "patch-package"
},
npx patch-package --exclude 'nothing' @ffmpeg/util
E.g // @ffmpeg+util+0.12.1.patch
diff --git a/node_modules/@ffmpeg/util/package.json b/node_modules/@ffmpeg/util/package.json
index 3afedb4..0a48d3a 100644
--- a/node_modules/@ffmpeg/util/package.json
+++ b/node_modules/@ffmpeg/util/package.json
@@ -3,6 +3,7 @@
"version": "0.12.1",
"description": "browser utils for @ffmpeg/*",
"main": "./dist/cjs/index.js",
+ "type": "module",
"types": "./dist/cjs/index.d.ts",
"exports": {
".": {
Hope that helps someone! |
I'm using Vite and pnpm and when trying to use @ffmpeg/util I'm getting this error:
Adding
"type": "module"
to@ffmpeg/util
'spackage.json
fixes the error 👍