cheton / browserify-css

A Browserify transform for bundling, rebasing, inlining, and minifying CSS files.
http://cheton.github.io/browserify-css/
MIT License
144 stars 22 forks source link

Inline images #47

Closed damoclark closed 7 years ago

damoclark commented 7 years ago

Hi,

Thanks for creating this library.

Wondering if you would accept this PR for adding support for inlining images into the css files?

README.md has been updated, and a test case has been included.

Damo.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+3.3%) to 84.27% when pulling c97c44d64ccf3f5f0d00ed597e97aa53a706e5e5 on damoclark:inlineImages into 78a597c120244c03a1a7d9e6f50d7ed8e87c950b on cheton:master.

cheton commented 7 years ago

Hi @damoclark,

Thanks for your PR. There is an example in the README.md that shows how to use processRelativeUrl() to inline image. May I know if that works for you?

https://github.com/cheton/browserify-css#processrelativeurl

var _ = require('lodash');
var path = require('path');
var browserify = require('browserify');

browserify(options)
    .add('src/index.js')
    .transform(require('browserify-css'), {
        rootDir: 'src',
        processRelativeUrl: function(relativeUrl) {
            if (_.contains(['.jpg','.png','.gif'], path.extname(relativeUrl))) {
                // Embed image data with data URI
                var DataUri = require('datauri');
                var dUri = new DataUri(relativeUrl);
                return dUri.content;
            }
            return relativeUrl;
        }
    })
    .bundle();
damoclark commented 7 years ago

Hey @cheton

Admittedly I didn't try that feature. The reason for building this functionality into the browserify-css API was so it could be used without having to write custom code to do it each time I want to use it in a project. Even though the code is quite simple, it's lower-level than what I was hoping for, which makes re-use inconvenient. So instead, I was hoping to be able to create a browserify pipeline from the command line for instance like the following excerpt from a package.json file:

{
  "scripts": {
    "browserify": "browserify -t [ browserify-css --inlineImages=true --autoInject=false ] -t imgurify -p [ browserify-header --raw --file www/userscript.meta.js ] js/index.js -o www/userscript.user.js",
  }
}

I'm attempting to craft a simple workflow to make writing userscripts much easier by leveraging the packagement management of npm, and browserify. Ultimately I want to be able to bundle a userscript into a single file, and using require() to pull artefacts such as images and css into variables that can then more easily be injected into the DOM from the userscript space. Inlining images into css is very convenient in this context, and something that a userscript author would want to do frequently. Or at least its something I want to do frequently. :)

The above package.json excerpt using npm run browserify takes a file containing a userscript metadata header www/userscript.meta.js and prepends it to the script proper js/index.js, allows images to be require()ed into the code and injected into pages as data-uris, and with my PR, I can similarly inline image urls in css files before pulling them in with require() and inject into the page.

Having said all that, I'm hardly an expert with browserify, and I'm still learning. Is there an easier way to integrate this functionality than my PR?

Thanks again @cheton

Cheers, Damo.

cheton commented 7 years ago

It make sense to provide an option to inline image directly without writing too much code, but I'd like to make the configuration more flexible just like webpack's url loader. For example, adding a limit option to inline image only if the file is smaller than a byte limit.

Webpack URL Loader

https://github.com/webpack/url-loader#usage http://webpack.github.io/docs/using-loaders.html#query-parameters

The url loader works like the file loader, but can return a Data Url if the file is smaller than a byte limit. The limit can be specified with a query parameter. (Defaults to no limit)


In addition to a simple boolean value to inline all images, the inlineImages option can also be an object in the following form:

"inlineImages": {
    "options": {
        "limit": 0 // Defaults to no limit
   }
}

Could you help update your PR with such flexibility? That will help a lot. Thank you!

damoclark commented 7 years ago

Hi @cheton

This makes sense. I can change the PR accordingly. What is your thinking on the default without specifying options to inlineImages? Should the default be 'no limit' unless a limit is provided?

cheton commented 7 years ago
  1. The inlineImages option will default to false if not specified.
  2. When inlineImages is enabled with a value of true or an object, defaults to no limit if 'limit' is not specified.
coveralls commented 7 years ago

Coverage Status

Coverage increased (+3.8%) to 84.783% when pulling 54ac3045d7739374ec623c0f31fe8ef90d838735 on damoclark:inlineImages into 78a597c120244c03a1a7d9e6f50d7ed8e87c950b on cheton:master.

damoclark commented 7 years ago

Hi @cheton

I updated the PR as discussed. I just realised I didn't put the actual limit in the test description in test/index.js. The limit in the test was 100K.

Make any changes to the PR you feel necessary.

Damien.

cheton commented 7 years ago

I will take a look at it. Thank you!

cheton commented 7 years ago

Released 0.10.0