LongTengDao / j-toml

A Node.js implementation of TOML written by LongTengDao. Belong to "Plan J".๏ผ้พ™่…พ้“ไธบๆฑคๅฐๆ˜Ž่ฏญๅ†™็š„ Node.js ๅฎž็Žฐใ€‚ไปŽๅฑžไบŽโ€œ็ฎ€่ฎกๅˆ’โ€ใ€‚
https://npmjs.com/package/@ltd/j-toml
GNU Lesser General Public License v3.0
55 stars 6 forks source link

v1.16.0 breaks bundled javascript #8

Closed bglw closed 3 years ago

bglw commented 3 years ago

Hello ๐Ÿ‘‹

v1.16.0 introduces a static fs import in parse/.ts:8. This breaks any frontend bundling that isn't stubbing out node libraries. In a default esbuild run I now get:

 > node_modules/@ltd/j-toml/index.mjs:1:32: error: Could not resolve "fs" (use "platform: 'node'" when building for node)
    1 โ”‚ import { readFileSync } from 'fs';

Could this be changed to something like a dynamic import behind a try? Loosely something like:

let fs = { readFileSync: () => { throw Error("readFileSync unsupported outside of node"} } };
try {
    fs = await import('fs');
}

Or something behind a platform flag.

LongTengDao commented 3 years ago

I see.

Or something behind a platform flag.

How?

bglw commented 3 years ago

Hmm, I was under the impression there was a supported way to check but it doesn't look like there is.

The checks in the browser-or-node package look simple enough to re-implement, though.

bglw commented 3 years ago

In saying that, I don't know if other packages provide any workaround for this, so maybe it's idiomatic for users to fix this in their bundler configuration... ๐Ÿค”

LongTengDao commented 3 years ago

I don't know any way good enough. I removed the statically module loading, hope that would help?

bglw commented 3 years ago

Ah, that's perfect. Thanks a tonne! ๐Ÿ˜„ ๐ŸŽ‰