search

mape / connect-assetmanager

Middleware for Connect (node.js) for handling your static assets.
MIT License
310 stars 48 forks source link

Corrupts bootstrap.js #33

Open tig opened 12 years ago

tig commented 12 years ago

Using connect-assetmanager in a node.js/Express app.

This works:

js: {
  dataType: 'javascript',
  path: __dirname + '/../public/javascript/',
  files: [//'jquery-1.7.1.js'
         'bootstrap.min.js'
  ],
  route: /\/static\/javascript\/script\.js/
}

(Note using the already minimized version of boostrap.js, and commented out the jquery file for testing).

This fails:

js: {
  dataType: 'javascript',
  path: __dirname + '/../public/javascript/',
  files: [//'jquery-1.7.1.js'
         'bootstrap.js'
  ],
  route: /\/static\/javascript\/script\.js/
}

Note the non-minimized bootstrap.js.

The failure is in the browser (Chrome). At line 120 in the connect-assetmanager compressed portion of bootstrap.js there's a syntax error:

119 isActive=$parent.hasClass('open')
120 clearMenus()!isActive&&$parent.toggleClass('open')
    Uncaught SyntaxError: Unexpected identifier
121 return false}}

Shocked by this, as I would have expected bootstrap.js to be a super common use case for connect-assetmanager. Happy to be shown that i've done something wrong.

klaemo commented 12 years ago

I'm pretty sure Bootstrap's rather stupid use of ASI is to blame here as it tends to screw up the minification process. I'm just using the minified version.