Skip to content
This repository has been archived by the owner on Mar 17, 2021. It is now read-only.

Add ability to set encodings #25

Closed
kisenka opened this issue Jan 29, 2016 · 5 comments · Fixed by #210
Closed

Add ability to set encodings #25

kisenka opened this issue Jan 29, 2016 · 5 comments · Fixed by #210

Comments

@kisenka
Copy link

kisenka commented Jan 29, 2016

Hi @sokra!
Thanks for the great loader.

Default loader behaviour is to encode source and return Data URI. But for SVG is no necessary to encode source in base64, because it works with raw content encoded with encodeURIComponent. Moreover it will decrease ~30% of content length and save browser resources for unpacking base64 string. My suggestion is to add user defined handlers to specific mime-type, something like this:

{
  module: {
    loaders: [
      {
        test: /\.(jpg|png|gif|svg)$/
        loader: 'url?limit=10000'
      }
    ]
  },
  url: {
    handlers: {
      'image/svg+xml': function (content) {
        return 'data:image/svg+xml,' + encodeURIComponent(content);
      }
    }
  }
}

For backward compatibility option can be disabled by default. For details please take a look at my advanced-url-loader which based on url-loader. I can make a PR, if you agree. Also I want be a volunteer and cover all existing loader code with tests. And close some issues.

@joekrill
Copy link

IE has issues with Base64 encoded data URIs, so I'm having problems related to this also. But I'm not sure it even needs to be as complicated as you suggest. A simple option to toggle using URL-encoded plaintext instead of Base64 encoding should be sufficient. Those are the only valid way to encode data URIs anyway (see http://codepen.io/tigt/post/optimizing-svgs-in-data-uris). Then you could just do something like:

loaders: [{ 
    test: /\.(svg)$/,
    loader: 'url?limit=10000&encoding=url'
}, {
   test: /\.(png)$/,
   loader: 'url?limit=10000&encoding=base64'
}]

@kisenka
Copy link
Author

kisenka commented Feb 16, 2016

@joekrill good suggestion

@michael-ciniawsky
Copy link
Member

@kisenka Sry for the delay are you still interested in a solution here ? @joekrill's comment sounds promising (add options to set encoding)

@michael-ciniawsky
Copy link
Member

@kisenka @joekrill PR highly welcome after #70 lands 😛

@michael-ciniawsky michael-ciniawsky changed the title Add ability to specify mimetype-specific encoding strategy feat: add ability to set encodings Apr 9, 2017
@michael-ciniawsky
Copy link
Member

#28 needs to be reworked (WIP)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.