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

Added type=module to @ffmpeg/util package.json #627

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

asticode
Copy link

I'm using Vite and pnpm and when trying to use @ffmpeg/util I'm getting this error:

import { ERROR_RESPONSE_BODY_READER, ERROR_INCOMPLETED_DOWNLOAD, } from "./errors.js";
^^^^^^

SyntaxError: Cannot use import statement outside a module
    at internalCompileFunction (node:internal/vm:73:18)
    at wrapSafe (node:internal/modules/cjs/loader:1178:20)
    at Module._compile (node:internal/modules/cjs/loader:1220:27)
    at Module._extensions..js (node:internal/modules/cjs/loader:1310:10)
    at Module.load (node:internal/modules/cjs/loader:1119:32)
    at Module._load (node:internal/modules/cjs/loader:960:12)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/translators:169:29)
    at ModuleJob.run (node:internal/modules/esm/module_job:194:25)

Adding "type": "module" to @ffmpeg/util's package.json fixes the error 👍

Copy link

netlify bot commented Nov 12, 2023

Deploy Preview for ffmpegwasm canceled.

Name Link
🔨 Latest commit d76af56
🔍 Latest deploy log https://app.netlify.com/sites/ffmpegwasm/deploys/6550a9f339a8440008200adb

Copy link

@nberlette nberlette left a 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",

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.

Copy link
Collaborator

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!

@PaulieScanlon
Copy link

Just a little +1 from me. I'm seeing the same error when using Remix.

@PaulieScanlon
Copy link

I ran into the same problem. If you need a work around, this is what worked for me.

  1. Install patch-package
npm install patch-package --save -dev
  1. Add postinstall script to package.json
// package.json

  "scripts": {
    ...
    "postinstall": "patch-package"
  },
  1. Run patch-package with the --exclude flag
npx patch-package --exclude 'nothing' @ffmpeg/util
  1. Commit the changes
    You should then see a new .patch file is created in /patches

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!

@jeromewu jeromewu added the triage Investigating issue label Dec 17, 2023
@piyush-panpaliya
Copy link

any update on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage Investigating issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants