Skip to content
This repository has been archived by the owner on Jan 14, 2020. It is now read-only.

Optimize method saveDataURIAsFile #511

Open
franckc opened this issue Sep 20, 2018 · 3 comments
Open

Optimize method saveDataURIAsFile #511

franckc opened this issue Sep 20, 2018 · 3 comments
Assignees
Labels
ipfs InterPlanetary File System

Comments

@franckc
Copy link
Contributor

franckc commented Sep 20, 2018

The method ipfsService.saveDataURIAsFile seems sub-optimal.
I haven't had time to test thoroughly on Node and Browser but we should be able to replace it with something that does not involve converting every single byte into an Uint8Array.
Something like this:

 async saveDataURIAsFile(dataURI) {
    let binary
    if (typeof Blob === 'undefined') {
      binary = new Buffer(dataURI.split(',')[1], 'base64')
    } else {
      const mimeString = dataURI
        .split(',')[0]
        .split(':')[1]
        .split(';')[0]
      const data = new Buffer(dataURI.split(',')[1], 'base64')
      binary = new Blob([data], {type: mimeString})
    }
    return await this.saveFile(binary)
  }

While doing that, we should add some unit test on the back-end for uploading Data URI.
Here is a snippet for creating a Data URI for an image:

      const imageBin = fs.readFileSync(imagePath)
      const imageBase64 = new Buffer(imageBin).toString('base64')
      const contentType = imagePath.endsWith('jpg')
        ? 'image/jpeg'
        : 'image/png'
      const medium = {
        url: `data:${contentType};base64,${imageBase64}`,
        contentType: contentType
      }
@franckc franckc self-assigned this Sep 20, 2018
@tomlinton
Copy link
Contributor

I remember working on that and having to do some strange things to maintain both node and browser compatibility. And also maintain support across different browsers. I don't remember the exact details but I'd suspect what I've done was mostly for compatibility reasons. I'm sure it can be improved though!

@franckc
Copy link
Contributor Author

franckc commented Sep 20, 2018

I tested my suggestion on both Node and Chrome browser. But I agree more testing on different type of browsers would make me feel more confident...

I'm actually not clear on why we need to use Blob on browser... Why not passing a Buffer to the saveFile() method in all cases ?

@tomlinton
Copy link
Contributor

I think browsers don't have Buffer?

@micahalcorn micahalcorn added this to Backlog in Origin Project Sprints via automation Sep 20, 2018
@micahalcorn micahalcorn added the ipfs InterPlanetary File System label Sep 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ipfs InterPlanetary File System
Projects
None yet
Development

No branches or pull requests

3 participants