New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Making this library isomorphic #38
Comments
Sure, why not. |
Please note that formdata-node itself requires Node 12.4 and newer. |
@octet-stream thanks for quick answer. Issuing the PR in couple of minutes.
Would be great to have this information in README. I'll incorporate this info in my PR as well. |
This is mentioned at |
Sure. Thank you 👍 |
Yeah, I dealt with this myself in the past. Decided to remove the To sum it up, |
PR issued #39 |
The new version is out, you can try it now: https://github.com/octet-stream/form-data/releases/tag/v3.6.0 |
Thanks! I'll leave this open for reference of: #39 (comment) |
Finished testing. The current solution doesn't work in webpack@5 due to this small intricacy:
In order to fix this we have to introduce mapping as proposed in my #39 (comment). Tested in locally (by direct override in node_modules) and it works like a charm. Issued a PR to fix it: #40 |
Let's try a new version: https://github.com/octet-stream/form-data/releases/tag/v3.6.1 |
Just noticed that Line 19 in 44a8ff9
|
Yep let’s point to CJS version of browser.js. I will act as legacy for old versions of bundlers that does not recognize ‘exports’ field |
Thanks for swift release; tests for webpack integration are currently running here: swagger-api/swagger-js#2154 |
Looks like one of tests failed because of async generators |
Which is strange because they seem to be available since 10.3 according to https://node.green/ |
Ah, I see, your tests run under Node 8. |
The changes we made to formdata-node browser mapping works like a charm.
Yeah, so far our CJS build artifacts worked even on Node 8. By introducing formdata-node CJS artifacts now require 12.4 as you mentioned earlier. But given that we plan to update node-fetch to v4 which is using formdata-node under the hood and requires 12.20.x we should be fine. |
Probably you meant node-fetch v3. Note that they will require ESM. See node-fetch/node-fetch#1141 By the way, you can still use formdata-node with node-fetch v2 if you will use |
Yeah, I mean v3 of course
Hm, we plan to use node-fetch@v2 with formdata-node@3.6.2. We directly pass |
Because node-fetch v2 does not have support for spec-compatible form-data implementations, they only handle
form-data-encoder will handle form-data encoding for you while you use an HTTP client in Node.js environment which does not handle spec-compatible FormData, so this is quite useful for node-fetch@2. Once you'll upgrade it to v3 you can just drop form-data-encoder. I think this needed to be mentioned too: node-fetch/node-fetch#1167 |
You can try send a request with formdata-node + node-fetch@v2 against https://httpbin.org/post with some file (or Blob/Buffer) and regular field. |
I assumed that they fully support formdata-node in v2. Here is a whole section describing that they do support it: https://github.com/node-fetch/node-fetch/blob/b50fbc105755123cad34fb0bc9d5653ecc693b8a/README.md#post-data-using-a-file-stream |
This is readme for v3 :) |
Right, thanks. So this means we have two options:
form-data-encoder option might seem like a way to go for now, and it doesn't need to be isomorphic as it will handle native FormData and FormData from this lib. |
form-data-encoder does not have any Node.js specific dependencies, so I think it can be bundled. But I haven't tested this use-case. If you'll encounter any problem with it - just file an issue there (in form-data-encoder repo) and we will figure out a fix :) |
Looks like using node-fetch@3 is going to be simpler in our case then using form-data-encoder with node-fetch@2. Anyway thanks for deep insight into all these libraries and how they work together. |
Alright then. |
Managed to successfully integrate formdata-node + form-data-encoder + node-fetch@v2 in swagger-api/swagger-js#2154. You'll probably see significant formdata-node downloads after merged and released ;] Yeah and thanks again for providing context for me during our conversations! |
Yeah, saw that last time you've tried to use my library in the past :D |
My colleague was handling it at that time and there was a time pressure. Now I had free Sunday and responsive library maintainer so it was pleasure ;] |
Hi,
Would you be open to make this library isomorphic by introducing the package.json browser field?
The
browser
field will point tolib/browser.js
file containing following code:This basically allows to use native
FormData
object from browser env. In any other case, the implementation ofFormData
object provided in/lib
will be used.If agreed I can issue a PR. The only effect for existing users will be that webpack and other bundlers will not be bundling this library any more (which they shouldn't do in the first place), but rather use
browser
field to getFormData
object.PS: I've seen people compensating for missing
browser
field couple of times already like here: https://github.com/tim-lai/isomorphic-form-data.The text was updated successfully, but these errors were encountered: