Pass minify config to jsminify task. #42

Merged
merged 3 commits into from Feb 5, 2013

Conversation

Projects
None yet
3 participants
@ghost

ghost commented Jan 29, 2013

I would like to pass "mangle" and "squeeze" option to jsminify task. The change here allowing mojito app to have such config in application.json:

[
  "settings": ["master"],
  "shaker": {
    "minify": {
      "config": {
        "mangle": true,
        "squeeze": true
      }
    }
  }
]

As per my test on our app, this help reduce script size from ~310K to ~220K in our case.

@caridy caridy commented on an outdated diff Jan 29, 2013

lib/shaker.js
@@ -673,7 +673,7 @@ Shaker.prototype = {
js: {task:['readResources', jsList]},
compileJS: {requires: 'js', task: ['preprocess']},
jsLint: {requires: 'compileJS', task: [shakerOptions.jslint ? 'jslint' : 'noop', {callback: jsLintCheck }]},
- jsMinify: {requires: 'jsLint', task: [shakerOptions.minify? 'jsminify': 'noop']},
+ jsMinify: {requires: 'jsLint', task: (shakerOptions.minify? ['jsminify', shakerOptions.minify]: 'noop'},
@caridy

caridy Jan 29, 2013

Collaborator
  1. Did you tried this code? It looks like there is a missing ) in this line. This you run this thru JSLint?
  2. when noop should be used, does it need to be an array? I'm not very familiar with the queue implementation.
Collaborator

caridy commented Jan 29, 2013

Can you also:

  • add a test if posible
  • add to the documentation
@ghost

ghost commented Jan 30, 2013

My bad, I didn't test this line. And queue.tasks also accept non-array parameter. See:
https://github.com/twobit/gear/blob/master/lib/tasks/tasks.js#L38-L47

@ghost

ghost commented Jan 30, 2013

I will improve my code to let it pass config to cssminify() as well. Will update pull request later.

@ghost

ghost commented Feb 1, 2013

Updated docs. And now I change config format like this to leave room of css minify options:

[
  "settings": ["master"],
  "shaker": {
    "minify": {
      "css": null,
      "js": {
        "mangle": true,
        "squeeze": true
      }
    }
  }
]

@caridy Can you show me how to run Shaker tests? So that I can add tests for this change.

Contributor

diervo commented Feb 5, 2013

You can create a folder under test called tasks. Test are standard YUITest files, copy one a follow the convention.
For running a test is just yuitest .

Shaker does not estrictly run all tests before bumping to a new version, so your pull request should be good.

+1

@aljimenez aljimenez added a commit that referenced this pull request Feb 5, 2013

@aljimenez aljimenez Merge pull request #42 from yiwu/master
Pass minify config to jsminify task.
24c8ffb

@aljimenez aljimenez merged commit 24c8ffb into YahooArchive:master Feb 5, 2013

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