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

Error when packing webm-writer with Webpack #16

Open
HerrZatacke opened this issue Jul 12, 2020 · 4 comments
Open

Error when packing webm-writer with Webpack #16

HerrZatacke opened this issue Jul 12, 2020 · 4 comments

Comments

@HerrZatacke
Copy link
Contributor

HerrZatacke commented Jul 12, 2020

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
Copy link
Owner

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
Copy link
Contributor Author

HerrZatacke commented Jul 13, 2020

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
Copy link
Contributor Author

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

@Cobertos
Copy link

Cobertos commented Dec 6, 2020

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.

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

No branches or pull requests

3 participants