Skip to content
This repository was archived by the owner on Nov 19, 2018. It is now read-only.

Node.js express example - #1

Merged
rnicholus merged 15 commits into
FineUploader:masterfrom
logickal:node_js_express_example
Apr 18, 2013
Merged

Node.js express example#1
rnicholus merged 15 commits into
FineUploader:masterfrom
logickal:node_js_express_example

Conversation

@logickal

Copy link
Copy Markdown
Contributor

Hi there! I've created a more robust (but also more up-to-date) server example for node.js and express. Definitely interested in your feedback - I believe that the current example is based on an older version of express and is now out of date. Additionally, I included front-end code to make it a fully-working example.

Thanks!

@rnicholus

Copy link
Copy Markdown
Member

Hi there Jeremy, and thanks for this contribution. Just a couple comments and one requested change:

  • It looks like you are returning a 404 status code in your error response. This will prevent Fine Uploader from reading the actual contents of the response in IE9 and older. All responses should be 200 (though 202 and 204 are also permitted for DELETE requests). Please see the 4th "Q" from the top on the Fine Uploader FAQ page for more details.
  • Historically, I have not included client-side code in the server examples folder/repo. I've done this to ensure that I don't wind up with stale, out-of-date code in the server examples folder/repo. The client-side interface/code changes often, relative to the server-side requirements. I can probably relax this restriction again and see if I can remember to keep this client-side portion of these examples up-to-date (if they exist). Looking at your client-side example, I would also like to add that I'm not familiar with jade templates, but that probably isn't a huge maintenance obstacle.
  • Since you have provided, it seems, a great deal of changes, feel free to add your name to the example code comments.

@logickal

Copy link
Copy Markdown
Contributor Author

Hi Ray!

Thanks for the feedback. I think I must have used the 404 from code I had based on the original example. Will change for sure.

On the client-side code, I understand you wanting to keep them separate, but since I was adapting this from a project I am working on, was concerned that removing too many moving parts would cause breakage, and also wanted to make sure I was capable of testing. Totally your call - if you would rather I pull the client side stuff and replace with comments, I could do that too.

Thanks again for looking at it - the newer versions of express make the integration SO much easier than what was given in the original example. I have been hunting high and low for a couple of weeks to find best way to implement, and it turns out it was much easier than I thought. Also, thanks for a great file upload tool!

@rnicholus

Copy link
Copy Markdown
Member

Jeremy - Thanks for the compliment!

You can go ahead and leave the client-side code in. I can always remove it or modify it later if I run into maintenance headaches. The 404 -> 200 adjustment, along with any mention of yourself in the code comments should be the only changes necessary. Also, can you please confirm that you've tested your example in IE9 or older, along with a File API browser (Chrome, Firefox, or Safari for OS X)?

@logickal

Copy link
Copy Markdown
Contributor Author

Made the appropriate adjustments, and can verify that it is tested and working in Safari, Chrome, Firefox and IE8.

@rnicholus

Copy link
Copy Markdown
Member

Looks good @logickal. Thanks again for your contribution!

rnicholus pushed a commit that referenced this pull request Apr 18, 2013
@rnicholus
rnicholus merged commit 3822b5e into FineUploader:master Apr 18, 2013
@logickal
logickal deleted the node_js_express_example branch April 18, 2013 18:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants