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

uglify breaks firebase.js #27

Open
joncostello opened this issue Jan 22, 2016 · 39 comments
Open

uglify breaks firebase.js #27

joncostello opened this issue Jan 22, 2016 · 39 comments

Comments

@joncostello
Copy link

Moved here from: Polymer/polymer-starter-kit#378

Steps to reproduce:

yo polymer
yo polymer:el test-firebase
bower install --save GoogleWebComponents/firebase-element

Insert a test-firebase tag in index.html(I put it above the my-greeting tag).

Inside test-firebase.html:
link firebase-document
above hello from test-firebase put a firebase-document tag

gulp serve:dist
Result:
blank page. Dev console has the Uncaught SyntaxError: Unexpected token ILLEGAL

@zboralski
Copy link

Same problem here...

<script language="JavaScript" type="text/javascript" src="../../bower_components/jsrsasign/jsrsasign-latest-all-min.js"></script>

with maximumCrush='true' I get :

Message:
    Expecting UnicodeEscapeSequence -- uXXXX

and maximumCrush='false' :

Message:
    Line 240: Unexpected token ILLEGAL
Details:
    index: 258234
    lineNumber: 240
    column: 2902
    description: Unexpected token ILLEGAL

I am using the following vulcanize task from the mobile app recipe:

// load polybuild
var polybuild = require('polybuild');

// 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'}));
});

@zboralski
Copy link

I tried creating a non-minified version of jsrsasign (polybuild only choke on the minified version) but it is a cross-dependency nightmare and gave up.

@zboralski
Copy link

This seems to be related :

mishoo/UglifyJS#544

@zboralski
Copy link

I don't think uglify is the problem.... If I comment out this line :

  .pipe(crush ? uglify : leftAlign)

polybuild succeeds... but then my application crashes at runtime with the same error:

So I run HTMLPrettify on elements.build.js to see what line was causing the problem :

elements.build.js:9213 Uncaught SyntaxError: Unexpected token ILLEGAL
9213:   if (f--\x3e0) {

What I don't understand is where that \x3e0 comes from because the code in question is jsbn.js(http://www-cs-students.stanford.edu/~tjw/jsbn/)

and the original line is if (f-- > 0) { ... and the non-minified version is if(i-- > 0) {

@zboralski
Copy link

The problem is with vulcanize....

I ran vulcanize --inline-scripts app/elements/elements.html

and that f (f-- > 0) was changed to if (f--\x3e0)

@mm-gmbd
Copy link

mm-gmbd commented Feb 9, 2016

+1, same issue

EDIT: So, my problem appears to be different than @zboralski has explained, as mine is with firebase.js speficially.

After running Atom Beautify on firebase.js (firebase.js installed through bower), I was able to track down the problem (I think). On line 4716 of the source, I came across the following line:

this.Ga.src && "javascript:" === this.Ga.src.substr(0, 11) && (a = '<script>document.domain="' + document.domain + '";\x3c/script>');

However, after running through Polybuild, the resulting line in index.build.js is the following:

this.Ga.src && "javascript:" === this.Ga.src.substr(0, 11) && (t = '<script>document.domain="' + document.domain + '";

Notice the end of the line -- there is no \x3c/script> or </script>, and the string is not terminated! Not sure what tool under the hood is causing the issue, but that appears the be the issue.

EDIT 2: After manually changing the resulting index.build.js so that the above line is corrected, I receive a different error:

Uncaught SyntaxError: Unexpected end of input

It is the last line of the file, and I'm guessing because of the error above, the rest of the file is parsed incorrectly and therefore produces a complete dud.

@spirylics
Copy link

+1 firebase + polybuild (uglify) broken

@justinfagnani
Copy link
Contributor

This was fixed in vulcanize 1.14.7

@mm-gmbd
Copy link

mm-gmbd commented Mar 3, 2016

@justinfagnani, just ran again and have the same issue. Has the dependency for vulcanize been updated for polybuild?

@justinfagnani
Copy link
Contributor

No, but a fresh npm install should pick up the latest version.

@mm-gmbd
Copy link

mm-gmbd commented Mar 3, 2016

npm uninstall polybuild followed by npm install polybuild results in the same...

@justinfagnani
Copy link
Contributor

argh, npm is terrible :(

The constraints from polybuild -> gulp-vulcanize -> vulcanize should pick up 1.14.7

I'll have to file a request over at gulp-vulcanize to update their constraints.

@justinfagnani
Copy link
Contributor

Filed sindresorhus/gulp-vulcanize#36

@mm-gmbd
Copy link

mm-gmbd commented Mar 3, 2016

Awesome! Mind re-opening until that is updated considering Polybuild is still afflicted?

@justinfagnani
Copy link
Contributor

Sure thing

@justinfagnani justinfagnani reopened this Mar 3, 2016
@mm-gmbd
Copy link

mm-gmbd commented Mar 3, 2016

Also, just to make sure I'm not crazy, I'll manually update gulp-vulcanize in my own node_modules folder and see if it indeed works.

@justinfagnani
Copy link
Contributor

Make sure you're picking up vulcanize 1.14.7 in there

@mm-gmbd
Copy link

mm-gmbd commented Mar 3, 2016

So, I made sure that vulcanize in my node_modules folder was v1.14.7 (and gulp-vulcanize simply does require('vulcanize'), so it should be the right version). But, the resulting files are still broken, and they are broken the same way as before (as described in my first comment above).

@samcarecho
Copy link

Hi.

Any good news about the issue?

I've tried to use the follow code to remove firebase.js from being vulcanized, but it's not removing it.

gulp.task('vulcanize', function () {
  return gulp.src('dist/index.html')
    .pipe($.vulcanize({
      stripComments: true,
      inlineCss: true,
      inlineScripts: true,
      excludes: [path.resolve('./dist/bower_components/firebase/firebase.js')]
    }))
    .pipe(polybuild({maximumCrush: false}))
    .pipe(gulp.dest('dist/'));
});

How can I properly avoid firebase.js from being vulcanized?

@spirylics
Copy link

Hi,

It's not very clean, but it works, in waiting :

<script async>
            var script = document.createElement('script');
            script.async = true;
            script.src = 'firebase.js';
            document.head.appendChild(script);
</script>

@mm-gmbd
Copy link

mm-gmbd commented Apr 13, 2016

@spirylics, it looks like you have an extra set of closing brackets/parenthesis });, just wondering if this is an accidental addition or an accidental omission of something from the beginning.

@spirylics
Copy link

@mm-gmbd , It's an accidental addition :) Thanks

@evancaldwell
Copy link

I'm still having an issue with this. I'm on vulcanize 1.14.8 and gulp-vulcanize 6.1.0. I'm not sure what to do with @spirylics snippet above.

@mm-gmbd
Copy link

mm-gmbd commented Apr 26, 2016

@evancaldwell, I actually meant to follow up on this and never did. @spirylics, does the script you provided just get inserted somewhere in the index.html? I presume that it's position is important, but I'm with @evancaldwell and am not really sure (also, I haven't tested -- sorry!)

@spirylics
Copy link

This script posted above will replace your firebase script declaration. Like that uglify don't see firebase js and don't touch it !

@evancaldwell
Copy link

hmm.. @spirylics maybe my setup is different, I'm using Firebase polymer elements so the firebase script declaration is actually in bower_components/firebase-element/firebase.html

@spirylics
Copy link

Ok I see. In your case you can use importHref from Polymer utility fonctions : https://www.polymer-project.org/1.0/docs/devguide/utility-functions.html

The idea is to import dynamically firebase (script or html) to hide it of vulcanize / uglify...

@evancaldwell
Copy link

@spirylics sorry for my noobness, a couple further questions: should I be using importHref in my index.html? Also won't I still get an error after everything is vulcanized because I'm still importing the firebase elements that are causing the error?

@samcarecho were you ever able to fix up the gulp task to exclude firebase.js?

@spirylics
Copy link

Yes you can use importHref in your index.html. Just, you have to use firebase-element when firebase-element.html is loaded ; so use onload argument of importHref.

@samcarecho
Copy link

@evancaldwell the project on which I was working is using polybuild.
I did not manage to exclude firebase from it.

@dChin84
Copy link

dChin84 commented Dec 13, 2016

@mm-gmbd I am experiencing the same issue - Invalid or unexpected token when vulcanized. I have updated the version of vulcanize etc. Did you manage to resolve?

@mm-gmbd
Copy link

mm-gmbd commented Dec 13, 2016

@dChin84 - unfortunately, I never resolved this on my end. I couldn't track down the underlying tool that was causing the parse error, so I ended up just leaving. However, it's been so long now that Firebase has released an entirely new set of libraries/SDKs (v3), does that mean this issue persists into v3 (or are you still using a v2 lib)?

@dChin84
Copy link

dChin84 commented Dec 13, 2016

@mm-gmbd Thanks for your response. It's looking like the issue is still present I am using v3 of the SDK :(

@samcarecho
Copy link

samcarecho commented Dec 13, 2016 via email

@mm-gmbd
Copy link

mm-gmbd commented Dec 13, 2016

@samcarecho - I'm with you. This should have been fixed a long time ago, especially considering that both of these are Google products/projects...

@dChin84
Copy link

dChin84 commented Dec 14, 2016

I managed to work around the issue by modifying polymerfire removing the references to firebase so they don't get vulcanized just allowing polymerfire to be vulced and then including firebase as script references within the main page. Google please fix as this is far from a decent solution

@fernandezpaco
Copy link

Any plans to solve this issue? It is affecting production

Thanks

@justinfagnani
Copy link
Contributor

@fernandezpaco we've removed uglify in favor of Babli from Polymer CLI. That change will be out in the next release. If you're still using polybuild, we recommend switching to the CLI or the polymer-build library, which is what powers the CLI.

@jariaspe
Copy link

Hi @justinfagnani,

I work with @fernandezpaco and we've made a POC including the rxjs.lite.min.js in a new app created from scratch with Polymer-cli. We just include the script in my-app.html.

We've included a gulp task to build it as it's explained in generator-polymer-init-custom-build.

We still have the same problem that @zboralski reported. Inside rxjs.lite.min.js there's a this._r-->0 which is replaced by this._r--\x3e0 and breaks the execution.

In the current vulcanize task (now renamed to polymer-bundler) we could narrow the problem down to the Uglify2JS run by inlineScripts in this line.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants