octet-stream / form-data

Spec-compliant FormData implementation for Node.js
https://www.npmjs.com/package/formdata-node
MIT License
142 stars 17 forks source link

`browser` field #47

Closed char0n closed 3 years ago

char0n commented 3 years ago

I've noticed that the browser field is defined in following way:

"browser": "./lib/cjs/browser.js"

Wouldn't it make more sense to point the field to ./lib/esm/browser.js? I know the file doesn't have any imports, so it doesn't really matter if it's CJS or ESM, but looks odd to be pointing to CJS.

octet-stream commented 3 years ago

Browser field is here for backwards compatibility only and will be removed as I move to ESM only (near Node.js v12 EOL). I also have module field that points to esm version of this module, for bundlers.

char0n commented 3 years ago

Sure, but for example in webpack, browser field gets priority over module field - https://webpack.js.org/configuration/resolve/#resolvemainfields. This effectively means that CJS version of formdata is being bundled instead of ESM one.

octet-stream commented 3 years ago

Don't Webpack read exports first?

char0n commented 3 years ago

Yes, webpack@5 reads exports, and then fallback to browser. webpack@4 (everything running on create react app) uses browser field and ignores exports field.

char0n commented 3 years ago

It's no big deal, just IMHO pointing browser field to ESM build makes more sense as webpack@4 is still very widely used. I can issue a PR.

octet-stream commented 3 years ago

I don't really see any reason for this change if webpack can deal with it. Browser field will be removed anyway in the nearest future and there's almost no content in browser.ts

char0n commented 3 years ago

Right, I've just pointed to this and thought that it might be oversight.