-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
Adding HTML form data handling to core #38943
Comments
@bmeck You proposed adding |
I think the key question to answer is what would add to have this in core. Maintaining things in core is harder than maintaining them in the ecosystem and it adds a burden to a limited set of folks. |
MIME is as stable and important as HTML itself. The implementation should only provide minimal functionality like parsing and stream handling. This allows the community to add additional functionality on top of it. Currently, this is reimplemented every time. I think the maintenance burden is very low until the module is stabilized. |
I did, the PR still is open with a similar pushback.
…On Sun, Jun 6, 2021, 5:27 AM Dustin Deus ***@***.***> wrote:
I think the key question to answer is what would add to have this in core.
Maintaining things in core is harder than maintaining them in the ecosystem
and it adds a burden to a limited set of folks.
MIME is as stable and important as HTML itself. The implementation should
only provide minimal functionality like parsing and stream handling. This
allows the community to add additional functionality on top of it.
Currently, this is reimplemented every time.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#38943 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABZJI6BJVOTWE4EPHSXSODTRM5QZANCNFSM46E5CNFA>
.
|
Do you mean #21128? Maybe I miss something but I don't propose mime type handling but MIME multipart parsing. |
(off-topic: that PR is the oldest one open :) |
I'm +1 on continuing to try and get a MIME module into the runtime based on WHATWG spec, that said I think that is a little bit lower level than a full multipart form constructor / parser. I'm not 100% that we should support the a Form constructor / parser ourselves tbh... although if we were going to explore this I feel like starting in unidici (the next generation http client) might be a good place to focus @nodejs/undici thoughts? edit: just adding context... I'm not convinced right now that form creation / parsing is a consistent level of abstraction in our platform. We don't offer parsing / creation for other standard data formats (e.g. XML) and while it would be great to have a single implementation for our ecosystem to use, I don't know that the maintenance cost is worth it at the moment. If we do implement fetch it might be worth implementing the FormData constructor from the WHAT WG spec, although we should explore if this can be handled well in userland. undici-fetch doesn't implement form-data, that might be a good place to explore implementing the spec and figuring out what parts are missing from core to make this easy such as Blob and Mime |
Node.js positioned itself as "Node.js designed to build scalable network applications". Sending files from the client to the server is daily business. To explain to a new user that this can't be done easily with Node.js is crazy. I experienced a very bad experience by writing a new abstraction for https://github.com/fastify/fastify-multipart. I think it can be very valuable to provide a thin module that is only responsible to parse forms. Similar to https://golang.org/pkg/mime/multipart/#example_NewReader If we can't benefit from the core, it would be also fine to host a module under the Node.js organization as with undici and reference it in the docs. I think the idea to extend the standard library with regular npm modules very attractive. This is the same strategy we are doing with fastify. House current, high-quality core modules to the community. Btw: For example, all rust standard modules are crate packages. This can simplifying contribution too. |
We talked about this briefly in the TSC meeting today, but didn't get too far without @mcollina and @MylesBorins. However, it seems that it ties into the larger issue of how to decide what goes in core at nodejs/TSC#1041 which is also being discussed by the TSC. |
FWIW, I have implementation of (in case the connection is not clear... fetch uses the Body mixin, which includes a method for reading the body of a response as |
Removed agenda label until nodejs/TSC#1041 is resolved |
Implementing FormData requires that we add the File class (#39015) first. Append/Set a Blob entry requires you to upgrade the Blob to a File instance so the FormData can be serialized properly later. I definitely think we should add it to core as well. It can be shipped before fetch |
fyi, there are two other spec compatible FormData in the userland/npm
node-fetch plans to remove support for form-data |
node-fetch@3 just got support for decoding FormData and URLSearchParams payloads back to a FormData instance 🎉 fd = await new Response(new FormData()).formData()
fd = await new Response(new URLSearchParams()).formData() Now you don't really need formidable, busboy or anything like it to decode multipart payloads, you can just do something like this instead: import { Response } from 'node-fetch'
http.createServer(async function (req, res) {
const formData = await new Response(req, { headers: req.headers }).formData()
const file = formData.get('avatar')
const arrayBuffer = await file.arrayBuffer()
const text = await file.text()
const whatwgReadableStream = file.stream()
}) |
There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment. For more information on how the project manages feature requests, please consult the feature request management document. |
Can we conclude that FormData is now supported by the node built-in |
|
You are right about that. I would maybe not use it either for large uploads as it's right now (but it's possible to change that). but for smaller payloads where file isn't uploaded then i would use it for thing such as url encoded payload for small simple forms. There is also one thing in the blob behavior that i have been meaning to add into
It's a bit surprising that neither Deno and NodeJS haven't understood this and implemented it this way. b/c right now they are pretty useless in the way that Blobs can only be constructed only using allocated data from the memory and not be refered to some place on the hard drive or even getting a new File instance backed up bt the hard drive. If new large files parsed from
what NodeJS needs is #37340 to make this somewhat feasible/usable |
There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment. For more information on how the project manages feature requests, please consult the feature request management document. |
|
Forgive me for digging up an old issue, but the history is relevant. While form creation is in Node now via |
Is your feature request related to a problem? Please describe.
Provide a convenient way to parse and create forms according to https://datatracker.ietf.org/doc/html/rfc7578 and https://xhr.spec.whatwg.org/#interface-formdata
Describe the solution you'd like
Implementation examples:
Describe alternatives you've considered
Form constructing
Form parsing
Adding new features to the core is a controversial topic. The procedure in #19308 (comment) might help.
Yes in a performant way with good API design.
Is it widely used?
Yes, it is.
Does it have a clear spec?
Yes.
Is it likely to become obsolete in the near future?
No.
Does it need optimizations from the core?
Not necessarily.
Will the entire community benefit from it being part of the core?
Yes, it's essential to deal with forms in almost any app.
The text was updated successfully, but these errors were encountered: