origin-js icon indicating copy to clipboard operation
origin-js copied to clipboard

Optimize method saveDataURIAsFile

Open franckc opened this issue 7 years ago • 3 comments

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 avatar Sep 20 '18 03:09 franckc

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!

tomlinton avatar Sep 20 '18 04:09 tomlinton

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 ?

franckc avatar Sep 20 '18 05:09 franckc

I think browsers don't have Buffer?

tomlinton avatar Sep 20 '18 05:09 tomlinton