Coverage remained the same at 98.19% when pulling 7565661 on Obviuse:issue-485 into 55d90ce on form-data:master.

@coveralls any change that this can be merged?

Can I do anything to help getting this merged?

@3imed-jaberi Great! How can we get this merged? It's really annoying having to fix this manually each time.

Can I help in any way?

+1 for this being a great fix to include!

This issue is breaking my webpack build imports. Any chance we can get this merged in. I know it's been awhile but it's needed especially with Axios.

Can you please merge it ?

We're waiting on GitHub access then we can merge and release.

Would love for this to get merged in!

I'm confused. Why would lib/browser.js ever be running on a server? SSR runs with node paths, not browser paths.

@ljharb because people build components (e.g. React) or libraries without taking SSR in to consideration and use this lib as a dependency.

The PR is a simple fix for scenarios like that.

If a component isn't built for SSR, then the component simply can't be used on the server - but I'm still not clear on why the browser file would ever be invoked on a server. Are you bundling things on the server? (if so, don't do that)

@enkelmedia i've only recently become a maintainer, so i'm trying to be cautious.

Can you help me understand how lib/browser.js is being evaluated outside of a browser bundle? If there's a specific third-party component or tool causing that, which is it?

This PR does not break anything on your end, I created it years ago and I can’t remember exactly why. Sorry about that. It only checks if you are running in a browser context and if not - backing off.

I don’t see any problem from your perspective with accepting this - or could you explain why you are hesitant?

Note here that we are not living in a “optimal” world, someone with a great component might have a dependency on this package without understanding that it would impact SSR - the real issue is of course that they are using this package from the beginning - but a good solution would be for this package to not try to do anything in environments where it does not make sense.

This is what this PR is fixing.

I'm hesitant because it's not actually beneficial to paper over problems caused elsewhere.

Someone using this package should never be directly referencing the browser file - it only should be used via

In other words, this change doesn't make sense because the problem should never happen, and if it's happening, it should be fixed at the place that's doing the wrong thing - which isn't here.

You are right and I agree.

In theory.

Let’s hope people stop using this package and avoid the problem.

@ljharb this change is still required for any project that uses Vite.

Because Vite prefers the browser entry point instead of the main entrypoint.
https://github.com/vitejs/vite/blob/0fe95d4a71930cf55acd628efef59e6eae0f77f7/packages/vite/src/node/constants.ts#L42-L47

So, during SSR, the self object would be undefined and instead the window object gets accessed.

For now, I've been using this workaround:

import FormData from 'form-data/lib/form_data';

const resolveFormData = typeof window === 'object' ? window.FormData : FormData;
const form = new resolveFormData();

@vaibhavrajsingh2001 wouldn't you only use vite for your browser bundle? If it’s intended to use it for node, then that preference is incorrect.

@vaibhavrajsingh2001 wouldn't you only use vite for your browser bundle? If it’s intended to use it for node, then that preference is incorrect.

Vite is also used popularly for server side rendering.

Can we explore using the more modern conditional exports field in package.json (https://docs.npmjs.com/cli/v10/configuring-npm/package-json#exports). It's preferred by most modern tooling

So instead of

"main": "./lib/form_data",
"browser": "./lib/browser"

we could have

"exports": {
  "main": "./lib/form_data",
  "browser": "./lib/browser"
},

This would be a non-breaking change since the same items are being exported.

Unfortunately adding exports is basically always a breaking change; but either way, if vite is intended for server use, it shouldn’t be using the browser field at all in that scenario. I’d suggest filing a bug.

@ljharb could you explain how exports is basically always a breaking change?
I'd like to propose adding at least the following:

"exports": {
  "node": {
    "import": "./lib/form_data.js"
  }
}

Would that be breaking?
This fixes the SSR issue for me, and I've seen such a fix implemented across some repositories over the last few months.
Thank you for your work!

@dargmuesli yes. because any path not listed in exports is not reachable.

ESM can import CJS, so there's no need for that whatsoever. If you're using a tool that can't handle that, it's broken.