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

remove // webpack no include #39

Open
severo opened this issue Nov 21, 2024 · 3 comments
Open

remove // webpack no include #39

severo opened this issue Nov 21, 2024 · 3 comments

Comments

@severo
Copy link
Contributor

severo commented Nov 21, 2024

Find a way to avoid the comment here:

const fsPackage = 'fs' // webpack no include

@platypii
Copy link
Collaborator

platypii commented Nov 21, 2024

I wonder if we could fix this with a node-specific exports in package.json? So that asyncBufferFromFile only gets bundled if someone using it is targeting a nodejs environment, but not if targeted for the web?

https://nodejs.org/api/packages.html#conditional-exports

@severo
Copy link
Contributor Author

severo commented Nov 21, 2024

indeed, it seems a good idea, since import('fs') will fail in a browser.

@severo
Copy link
Contributor Author

severo commented Dec 10, 2024

Details in #49:

If you put await import('fs') then webpack looks at it and says "oh it looks like I need to bundle this too" but then fails because fs is a node system package not available in npm or the browser.

So the problem is not exception handling, it's the bundling.

I think that the solution is to have a conditional export. Possible that asyncBufferFromFile needs to be in its own file to avoid aggressive bundling, but not sure.
https://nodejs.org/api/packages.html#conditional-exports

I'll see if I can get a repro on the bundling issue. I think it happened when I imported from hyperparam or the cli without the string hack.

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 a pull request may close this issue.

2 participants