Skip to content
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

Should the filename pass to this._newFile also be renamed if this.options.filename === 'function' ? #671

Closed
edwardlai3582 opened this issue Jan 4, 2021 · 5 comments
Labels

Comments

@edwardlai3582
Copy link

Support plan

  • which support plan is this issue covered by? (e.g. Community, Sponsor, or
    Enterprise): Community
  • is this issue currently blocking your project? (yes/no): yes
  • is this issue affecting a production system? (yes/no): no

Context

  • node version: v10.17.0
  • module (formidable) version: master #69b9a6e
  • environment (e.g. node, browser, native, OS):
  • used with (i.e. popular names of modules):
  • any other relevant information:

What are you trying to achieve or the steps to reproduce?

In examples/store-files-on-s3.js, the filename received by uploadStream is still the part.filename since its the filename pass into this._newFile (

filename: part.filename,
)
Not sure its designed by intention or not

What was the result you got?

origin filename

What result did you expect?

renamed filename if option.filename exists

@tunnckoCore
Copy link
Member

I'm not very sure what you are trying.
Did you tried adding options.filename on that example?

I kind of sense what's the problem.. This option is more like "renaming the disk filepath" which.. works on "persistent mode", but not that much on volatile/exernal storage.
Hm.

@edwardlai3582
Copy link
Author

edwardlai3582 commented Jan 5, 2021

I'm not very sure what you are trying.
Did you tried adding options.filename on that example?

I kind of sense what's the problem.. This option is more like "renaming the disk filepath" which.. works on "persistent mode", but not that much on volatile/exernal storage.
Hm.

Thank you for the prompt reply.
Yes I'd like to use something else as s3Client.upload' Key and thought that I can just rename the filename with option.filename.
Guess I can just declare uploadStream in http.createServer (or is it an anti-pattern?)

@tunnckoCore
Copy link
Member

tunnckoCore commented Jan 5, 2021

Yes I'd like to use something else as s3Client.upload' Key and thought that I can just rename the filename with option.filename.

I understand. But it'a nor possible now I think. We should think something or better option.

Guess I can just declare uploadStream in http.createServer

I guess you can, yes. I don't see problem.

@tunnckoCore
Copy link
Member

I think we should pass part to _newFile(part { ... }) and then to VolatileFile constructor, So we can just pass the whole this and part to the fileWriteStreamHandler. I thought that why we just pass the filename which.. i'm still not sure what it means haha.

@davidstrouk
Copy link

+1, why don't we pass the whole object to fileWriteStreamHandler?
I want to return an Azure blob stream whose name is the hash of the file, so i would like to use this.hash as well. this.name here (filename) is not sufficient

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants