search

brillout / json-serializer

Same as JSON but with added support for Date, undefined, NaN, Infinity, RegExp and more.
MIT License
50 stars 5 forks source link

package according to Node.js spec #9

Closed benmccann closed 2 years ago

benmccann commented 2 years ago

Attempting to use this package in a Vite project results in:

@brillout/json-serializer doesn't appear to be written in CJS, but also doesn't appear to be a valid ES module (i.e. it doesn't have "type": "module" or an .mjs extension for the entry point). Please contact the package author to fix.

See https://publint.bjornlu.com/@brillout/json-serializer@0.5.1 and https://kit.svelte.dev/faq#packages

benmccann commented 2 years ago

I tested locally and it seems Vite is most upset about the . entry in exports. While the code is valid as either CJS or MJS, it can only be interpreted as CJS by Node since there's no .mjs extension on it.

There's several valid fixes here and I'm not sure which you'd like to pursue. I personally would add "type": "module" and get rid of the CJS code and the build step in this repo, but understand if you're not ready to go 100% ESM yet

brillout commented 2 years ago

Fix released in 0.5.2.

benmccann commented 2 years ago

It still looks to have issues: https://publint.bjornlu.com/@brillout/json-serializer@0.5.2

I think the browser related warnings are probably a bug in publint - FYI @bluwy. However, you only have *.js in files and I think you need *.mjs there as well

brillout commented 2 years ago

However, you only have .js in files and I think you need .mjs there as well

Indeed, thanks.

I think the browser related warnings are probably a bug in publint - FYI @bluwy.

Yea, I also consider these false positives. I get this warning for many projects, including vite-plugin-ssr, yet no one has complained about it. Bundlers seem to be fine with it; I've users using Vite, Parcel, and webpack.

bluwy commented 2 years ago

That makes sense, I create https://github.com/bluwy/publint/issues/3 to track it. Thanks!