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

Add compatibility for packing with webpack #17

Merged
merged 1 commit into from
Sep 30, 2021

Conversation

HerrZatacke
Copy link
Contributor

This should fix issue #16

I ran your tests npm run test as well which did work for me, but these only cover the electron-side as far as I can see, so please check if the "plain" browser version works for you as well.

@@ -220,7 +220,7 @@
};

if (typeof module !== 'undefined' && typeof module.exports !== 'undefined') {
module.exports = BlobBuffer(require('fs'));
module.exports = BlobBuffer;
} else {
window.BlobBuffer = BlobBuffer(null);
Copy link

@Cobertos Cobertos Dec 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels a little odd to me to have one platform exporting a function and the other exporting the return of the function. Maybe change main.js to call the function like browser.js does?

As a future feature, or maybe as part of this, I've also seen a pattern in other libraries to pass in fs as an external. It'd be nice if the user could just pass in whatever implementation of fs for whatever their needs are, meaning you could use anything from BrowserFS

@chrisgervang
Copy link

Any chance this (or something that solves the same issue) can be merged in? I've also encountered the issue with next.js visgl/hubble.gl#158

@ilijapuaca
Copy link

+1 for landing this in some capacity, we're stuck with the same issue when trying to make use of esbuild

@chrisgervang
Copy link

I linked this version of the library to the hubble.gl repo locally and ran the browser through some render tests. Great news! No runtime errors, and I was able to remove my webpack workaround. This seems like a great PR to merge and publish - much appreciated for putting it together @HerrZatacke, and thanks for your hard work @thenickdude!

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

Successfully merging this pull request may close these issues.

5 participants