thenickdude / webm-writer-js

JavaScript-based WebM video encoder for Google Chrome
272 stars 43 forks source link

Error when packing webm-writer with Webpack #16

Open HerrZatacke opened 4 years ago

HerrZatacke commented 4 years ago

Hey Guys,

I tried to include the webm-write with webpack, yet it tries to load the 'fs' (node) module on compile

I figured out (using webpack 4) you can add the following to your webpack config to prevent that error from happening.

  node: {
    fs: 'empty',
  },

Yet, I don't know if it'd possible to remove that requirement, maybe by passing fs as an optional parameter. This would allow people to add their own implementations of fs as well. (like fs-extra if desired)

Suggestion if you do not need fs:

const videoWriter = new WebMWriter({
  fs: null,
})

Or with a custom implementation:

const videoWriter = new WebMWriter({
  fs: require('my-custom-fs'),
})

Could be useful for some unit-tests as well...

thenickdude commented 4 years ago

Could you provide an example project that uses Webpack that I can use for testing? I don't use Webpack myself so I'm not sure how it would be set u p.

HerrZatacke commented 4 years ago

Sure thing :)

Here I prepared a working setup: https://github.com/HerrZatacke/webm-writer-webpack-demo

To get it working I had to add to my basic setup (https://github.com/HerrZatacke/webm-writer-webpack-demo/blob/master/scripts/webpack.common.js#L13)

node: {
  fs: 'empty',
}

This config is an okay-ish thing, but as webpack is resolving and packing all files that are referenced via require() from looking at this line in WebMWriter.js, the two files (ArrayBufferDataStream.js and BlobBuffer.js) are still being packed into the final production js. You can verify this by running npm run build in my repository and then check dist/main.bundle.js Edit: yes, they are, but actually should be as these are not ntive browser-functions :)

The preferred way would be, that these two calls to require() would not be present in a file that is meant to be packed by webpack.

I understand you went a different approach because you are using the lib mainly on the server (electron) side, which does not cause that problem. Yet webpack is (in my opinion, mostly the de-facto) standard when building web-applications, so allowing that would be much appreciated.

Maybe it could work using the browser property in your package.json. Which would mean you'd have to provide a separate file...

If you want, I can give it a try solving this issue?

HerrZatacke commented 4 years ago

Creating webpack-compatibility was easier than I expected. I'm using the above mentioned "browser" property in the package.json

Cobertos commented 3 years ago

Was looking for the same thing as well,

I ended up doing

config.module.rules.push({
  test: /webm-writer[\\/]/,
  loader: 'string-replace-loader',
    options: {
      search: 'require(\'fs\')',
      replace: 'null'
    }
});

would love to have 1st party support though.