balderdashy / sails

Realtime MVC Framework for Node.js
https://sailsjs.com
MIT License
22.84k stars 1.95k forks source link

When using Sails ≤ v0.12: file uploads displaying deprecation warning about "os.tmpDir()" #4326

Open sanjaykumarns opened 6 years ago

sanjaykumarns commented 6 years ago

Sails version: 0.12.14 Node version: v9.4.0 NPM version: 5.6.0 DB adapter name: N/A DB adapter version: N/A Operating system: Debian GNU/Linux 8 (jessie)


I am trying to implement file upload api in sailsjs (Followed this), but the DeprecationWarning is showing while dealing with file type.

(node:23566) [DEP0022] DeprecationWarning: os.tmpDir() is deprecated. Use os.tmpdir() instead.

Sailsjs version: 0.12.14

I have solved the deprecated warning by changing the 55th line in node_modules/sails/node_modules/skipper/node_modules/multiparty/index.js

from self.uploadDir = options.uploadDir || os.tmpDir(); to self.uploadDir = options.uploadDir || os.tmpdir(); But it is not a good solution as we are ignoring these node_modules folders while pushing into git and moving into production servers.

The question is available in stack overflow also: https://stackoverflow.com/questions/48990342/sailsjs-file-upload-is-not-working

sailsbot commented 6 years ago

@sanjaykumarns Thanks for posting, we'll take a look as soon as possible.


For help with questions about Sails, click here. If you’re interested in hiring @sailsbot and her minions in Austin, click here.

mikermcneil commented 6 years ago

@sanjaykumarns thanks for posting. We must have sorted this out in a subsequent release of Skipper, because I haven't seen it myself and we've been doing a lot of file upload work recently. But we're also using the current LTS version of Node (v8.x).

Would you mind trying this out in your app on Node 8, just to verify that this is a Node 9-only problem? If you're also seeing it on Node 8, then something else must be going on

VirtualWolf commented 6 years ago

@mikermcneil I see the same behaviour myself with my own application under Node 8.12.0 and Sails 0.12.4 on macOS 10.13.6 and was able to replicate it with a super-simple app as well: https://github.com/VirtualWolf/sails-file-upload-test

johnabrams7 commented 5 years ago

@sanjaykumarns @VirtualWolf - Thanks for all the help so far and sharing the examples. I see there are newer versions of Node 8 LTS (8.16.0) and Node 9 (9.11.2) that may help resolve this issue - has anyone seen any improvement after updating?

VirtualWolf commented 5 years ago

@johnabrams7 Just tested and it still occurs with both 8.16.0 and 9.11.2. (I'm now on macOS 10.14.4 if that matters).

mikermcneil commented 5 years ago

@VirtualWolf Thanks for that! (updated issue title accordingly)

@sanjaykumarns @VirtualWolf You know, I suspect what might have happened is that this deprecation warning started appearing with a different Node version we were developing with when working with our last remaining Sails v0.12 app last year.

Since this is just a warning, and since its no longer an issue in Sails v1, let's triage any further action on this. If anyone notices this occurring in Sails v1.0 or later though, please drop a comment in here -- I will definitely address that if it comes up.

Thanks again to you both!

parthg-wysa commented 2 years ago

Hi. this is currently failing on Sails 0.12.14 due to the same issue. I believe updating that to use Skipper 0.8.0 should fix it according to: https://github.com/pillarjs/multiparty/issues/164#issuecomment-279394122

stuk88 commented 1 year ago

This breaks sails 0.12. Needs an update to os.tmpdir in skipper files. Need a PR to make sails.js support skipper disk adapter.

eashaw commented 1 year ago

Hi @stuk88 what version of node are you using? Have you tried the workaround that @parthg-wysa posted above? (https://github.com/balderdashy/sails/issues/4326#issuecomment-965044183)

stuk88 commented 1 year ago

I did update the skipper version in the project, but the library itself needs a fork to load 0.8.0. So the skipper that is loaded from sails, is lower than 0.8.0