throw with a clearer exception if using node-only function in browser #49
Conversation
| * @param {string} filename | ||
| * @returns {Promise<AsyncBuffer>} | ||
| */ | ||
| export async function asyncBufferFromFile(filename) { | ||
| const fsPackage = 'fs' // webpack no include |
There was a problem hiding this comment.
I think this PR missed the reason for the "webpack no include" hack. 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.
There was a problem hiding this comment.
Ohhhh OK. I'll close this PR, and maybe look at this issue later, as it's not super urgent.
There was a problem hiding this comment.
As mentioned in the PR message, I'm not sure if conditional exports is the solution because a Node.js app can use either ESM imports or CJS require.
As node can now use ES modules, I understand we cannot really have a build for node and a build for browsers. This means we have to check the availability of the 'fs' module at runtime and throw if it's not available (browser).
Maybe my understanding is wrong, but I didn't find clear answers.
fixes #39.