multipart api

[tj]

something like:

request.post('/video')
  .type('mixed/related; boundary=---')
  .part()
    .set('Content-Type', 'application/xml')
    .data(metadata)
  .part()
    .set('Content-Type', 'video/webm')
    .set('Content-Length', stat.size)
    .data(fs.createReadStream(path))
  .end(callback)

auto-assign boundary when mixed/*:

request.post('/video')
  .type('mixed/related')
  .part()
    .set('Content-Type', 'application/xml')
    .data(metadata)
  .part()
    .set('Content-Type', 'video/webm')
    .set('Content-Length', stat.size)
    .data(fs.createReadStream(path))
  .end(callback)

support nesting

[c4milo]

do you want to go still with this API? I think it's too low level.

[tj]

build up my friend! up!

[tj]

the high level stuff like piping files etc can be layered on top easily

[tj]

minus .data() since that is now .send(), and should be .pipe() for multipart streams

[c4milo]

hehe, meh, I'm so user centric now. I usually design top down.

On Wed, Jan 4, 2012 at 1:03 PM, TJ Holowaychuk <
reply@reply.github.com

wrote:

build up my friend! up!

---

Reply to this email directly or view it on GitHub:
#11 (comment)

Camilo Aguilar
Software Engineer

[tj]

then you end up with inflexible APIs like request, obviously you have the high level API in mind, but that's definitely not the place to start building

[c4milo]

I agree, but then if you want to be pragmatic. I'd suggest you to release a simple and functional multipart support since it's pretty straight forward to do. Then, you can continue working on a better and more flexible implementation aka low level api and high level api on top. It's fine if you decide to go straight with the latter since I can use my fork in the mean time.

Just putting my 2 cents here.

[c4milo]

@visionmedia I'll be nice also to have the high level API like you envisioned it.

[tj]

that's probably what mikeal had in mind for request haha and look at it now, it's just one big function. just takes a bit of time that's all

[c4milo]

it was improve in node 0.6.x wasn't it?

[tj]

nope: https://github.com/mikeal/request/blob/master/main.js#L100

[c4milo]

hug

[tj]

haha :D this is why I started superagent, didn't want a rushed lame api

[c4milo]

I was giving it more thought to the need of a low level API and I honestly think that we are over engineering the solution. RFC 1867 is really simple and I don't think it will change in the near future. For instance I don't see the value of allowing users to specify custom headers for every part. Also content-type can be determine by superagent, either by the file extension or using libmagic. The only thing that makes sense to me, so far, is allowing to do piping when downloading binary data.

I'm still open to discussion though :)

BTW, the API that I'm proposing would look like:

request.post('/upload')
  .set('x-mycustomheader', 'value')
  .part('field1', 'value1') //once it finds the first part it assumes content-type: multipart/form-data defining the boundary
  .part('field2', 'value2') //a plain field
  .part('file', filePath) //a file path, this could be also an array of file paths
  .end(function(res) {
    //boo
  });

[tj]

im just saying there is more to an API than the highest-level. If you need to set a header field then you need a method for that, if you set a content-type then you need a method for that, if you set a content-type based on a filename the you need a method/mime lookup for that. These lower level APIs would be great for testing node apps, crafting multipart requests that reside in memory (node-canvas etc), and they're basically required by the higher level API anyway

[tj]

you could say the same thing about node's setHeader() vs writeHead() which much more flexible

[tj]

stream.pipe(part) would stream the raw contents to the Part, so you still set the header fields. part.pipe(stream) would use the stream.path to determine the content-type etc, and perhaps part.pipe(path) would create the fs.ReadStream for example. That's more what I would call "proper" abstraction, it's all things you need anyway

[c4milo]

hehe, I got it now, I thought you were proposing the API that most users will ultimately be using...

[tj]

nope nope

[c4milo]

how did you come up with content-type mixed/related?. It isn't mentioned in RFC 1867.

[tj]

"A multipart/related is used to indicate that each message part is a component of an aggregate whole. It is for compound objects consisting of several inter-related components - proper display cannot be achieved by individually displaying the constituent parts." - http://tools.ietf.org/html/rfc2387

[tj]

not sure why im not using multipart/form-data in part.js haha... wth

[tj]

one of those sleepy nights i guess

[c4milo]

oh boy hehe, are you serious? the scope of most rest libraries for file uploads is only rfc1867. If you want to support the whole mime standard I'd suggest to create a new project hehe.

[c4milo]

or write bindings for http://spruce.sourceforge.net/gmime/

[tj]

my example isnt even right "mixed/related" it's "multipart/related" hahaha

[tj]

just ignore me haha

[c4milo]

nah, no problem. I'm going to review what we have so far and try to make rfc1867 work with your current design, given that you didn't like my big function ^^. I'd say that it's a fair scope for superagent for now.

[c4milo]

or if you want to do it yourself and want to save time just grab pieces from https://github.com/c4milo/superagent/blob/master/lib/node/part.js. It is working and tested with node-formidable