googlearchive / polyclean

BSD 3-Clause "New" or "Revised" License
20 stars 7 forks source link

Polyclean's UglifyJS setup breaks firebase.js #4

Open addyosmani opened 9 years ago

addyosmani commented 9 years ago

Cross posted from the PSK repo.

To reproduce: 1) Import one of the polymer firebase elements in a psk app (or any app using polyclean) 2) Build the app. 3) Run the build in any browser.

Result: Uncaught SyntaxError: Unexpected token ILLEGAL

Reason: poly-clean's uglifyJS function is run by poly-build & it appears uglify breaks firebase.js which is included by the firebase elements.

A workaround is to use https://github.com/PolymerElements/polymer-starter-kit#if-you-require-more-granular-configuration-of-vulcanize-than-polybuild-provides-you-an-option-by and use the excludes option in vulcanize, i.e.:

.pipe($.vulcanize({
      stripComments: true,
      inlineCss: true,
      inlineScripts: true,
      excludes: [path.resolve('./dist/bower_components/firebase/firebase.js')]
    }))
addyosmani commented 9 years ago

Friendly ping @azakus as the Firebase specific issues here are being reported now in a few different places.

ghost commented 8 years ago

Hi @addyosmani I've followed your direction on workaround above, where I've excluded firebase.js from vulcanize. However, now I get an issue "Firebase not defined", I'm thinking because the firebase.js isn't there. Is there something I'm missing or maybe some suggestion you have?

In the meantime, I'm using AJAX to just pull the Firebase data, but sadly I lose the firebase-collection awesomeness (three-way data binding, etc).

ghost commented 8 years ago

Hi @vernalislabs Could you provide some code? I'm using the workaround and get three-way binding like a charm :)

zboralski commented 8 years ago

I have the same issue with one of my custom elements that doesn't use firebase.

zboralski commented 8 years ago

This line is causing the problem :

  <script language="JavaScript" type="text/javascript" src="../../bower_components/jsrsasign/jsrsasign-latest-all-min.js">
  </script>
[13:02:02] Error in plugin "polyclean"
Message:
    Line 205: Unexpected token ILLEGAL
Details:
    index: 49907
    lineNumber: 205
    column: 2902
    description: Unexpected token ILLEGAL
zboralski commented 8 years ago

If I used the non-minified version jsrsasign it works fine

zboralski commented 8 years ago

I am using the gulp task from the mobile app recipe:

// Vulcanize granular configuration
gulp.task('vulcanize', function() {
  return gulp.src('app/elements/elements.html')
    .pipe(polybuild({maximumCrush: true}))
    .pipe($.rename(function(file) {
      if (file.extname === '.html') {
        file.basename = file.basename.replace('.build', '');
      }
    }))
    .pipe(gulp.dest(dist('elements')))
    .pipe($.size({title: 'vulcanize'}));
});

How I can I exclude jsrsasign/jsrsasign-latest-all-min.js ?

marxtseng commented 8 years ago

modified as follows to solve this problem

from this:
/**
 * Uglify the stream.
 */
uglifyJs: function uglifyJs(options) {
    options = options || {};
    options.fromString = true;
    .....
}
change to:
/**
 * Uglify the stream.
 */
uglifyJs: function uglifyJs(options) {
    options = options || {};
    options.output = options.output || { inline_script: true };
    options.fromString = true;
    .....
}