compilation errors caused by rollup deleting minimized firebase semicolon

[MichaelRando]

I have a firebase project that rollup is failing to build. I determined that rollup was corrupting the auth.js file out of firebase by removing meaningful semicolon. Rolling back rollupjs to version 0.36.1 "fixed" me. You probably want to look at this. g+oe|0a.h[0] is the broken substr.
Minimized rollup.config.js follows:

import replace from 'rollup-plugin-replace'
export default {
    entry:  'node_modules/firebase/auth.js',
    dest:   './dist/auth.js',
    format: 'iife',
plugins:
   [
      replace({
         'process.env.NODE_ENV': JSON.stringify( 'production' )
      })
   ]
}

[kzc]

Your rollup config works fine with:

• [email protected]
• [email protected]
• [email protected]

There is a warning about firebase's use of eval, but it's just a warning.

[MichaelRando]

Sure, it creates an output file. I would prefer it to create a valid javascript file, instead. For example:
instead of string: 'g+oe|0a.h[0]', I like the original file's: 'g+oe|0;a.h[0]'

[kzc]

I ran the generated rollup output file through [email protected] as well as [email protected] and confirmed it is valid javascript file.

I did not see the string in question. Please verify the versions of the modules match the ones I listed above.

[kzc]

I see what you mean now. The rollup output contains:

y<<26)^(y>>>11|y<<21)^(y>>>25|y<<7))|0,g=g+(vb[b]|0)|0,g=f+(g+(c[b]|0)|0)|0,f=Wa,Wa=Eb,Eb=y,y=n+g|0,n=k,k=e,e=d,d=g+oe|0a.h[0]=a.h[0]+d|0;a.h[1]=a.h[1]+e|0;a.h[2]=a.h[2]+k|0;a.h[3]=a.h[3]+n|0;a.h[4]=a.h[4]+y|0;a.h[5]=a.h[5]+Eb|0;a.h[6]=a.h[6]+Wa|0;a.h[7]=a.h[7]+f|0;};

Must be some issue with line wrap.

[kzc]

I had run the original input through uglifyjs -b initially which had worked.

[kzc]

Repro:

rollup version 0.41.4

$ cat r.js

for(var x=1;x<2;x++)var d=x|0;console.log(d);

$ rollup r.js -f es

for(var x=1;x<2;x++)var d=x|0console.log(d);

$ rollup r.js -f es | node

[stdin]:1
for(var x=1;x<2;x++)var d=x|0console.log(d);
                             ^^^^^^^
SyntaxError: Unexpected identifier

Possible fix:

--- a/src/ast/nodes/VariableDeclaration.js
+++ b/src/ast/nodes/VariableDeclaration.js
@@ -98,7 +98,7 @@ export default class VariableDeclaration extends Node {
                        const needsSemicolon = !forStatement.test( this.parent.type );
 
                        if ( this.end > c ) {
-                               code.overwrite( c, this.end, needsSemicolon ? ';' : '' );
+                               code.overwrite( c, this.end, needsSemicolon ? ';' : '\n' );
                        } else if ( needsSemicolon  ) {
                                this.insertSemicolon( code );
                        }

Either that or unconditionally introduce curly braces around the body of loops similar to how Bublé does it.

[MichaelRando]

could be a pull request?

[kzc]

Sure, go for it.

When you make a PR be sure to include the test above. See ffe6b0b for an example of a test.

[kylecordes]

I have encountered the same bug, in the same context of firebase. I'm trying to understand why this bug somehow leaves so much other code working correctly. Reading the source code (around the lines in the diff above), it sure seems like this would not be a very rare edge case.

[kylecordes]

Sed hackery, for anyone stuck by this bug:

# Remove me when fixed: https://github.com/rollup/rollup/issues/1275
sed -i.bak "s/needsSemicolon \? ';' : ''/needsSemicolon ? \';\' : '\\\\n'/g" node_modules/rollup/dist/rollup.js

This is an ugly, ugly thing to do, but it got my build process back up and going without having to roll back to a pre-October rollup.

[kzc]

I'm trying to understand why this bug somehow leaves so much other code working correctly.

The problem manifests itself more often when the input is minified. See: #1275 (comment)

[kylecordes]

@kzc Thanks - now I get it. It is an unusual coding pattern that hits this bug. Something a minifier would write, not something a human would write. Hence Firebase - which is distributed in minified form.

[MichaelRando]

still happens in 0.41.5. The broken auth statement is d = g + gh | 0a.h[0] = a.h[0] in auth.js of firebase 3.7.1

[kzc]

It still happens because no PR was made.

[MichaelRando]

I tried to use babili minifier with sourcemaps, which requires rollup > 0.41.5. But this bug is still happening in rollup 0.41.6. I think I'll have to try the sed command.

[hugomrdias]

this shouldnt be ignored ! firebase is unusable with rollup
should we go we this fix ? #1275 (comment)

[olsonpm]

As kzc mentioned, feel free to create a PR with that fix

[orbitbot]

I tried the suggested change in #1275 (comment) , now the setup compiles with importing firebase, however with my current setup I get

app.js:1601 Uncaught TypeError: Cannot read property 'navigator' of undefined
    at app.js:1601
    at app.js:1868
    at app.js:1870
    at commonjsGlobal (app.js:4)
    at app.js:5

during page load no matter if the fix is applied or not. This is with rollup 0.41.6 and buble.

[olsonpm]

please create a new issue with a reproduction

[orbitbot]

After banging my head against the wall for a while I realsed that this is an issue with the way the Firebase source refers to this in firebase/database in my case, so a workaround similar to #1007 (comment) seems to have solved the issue for now.

With import replace from 'rollup-plugin-replace' in a rollup.config.js

    replace({
      include: '**/firebase/database.js',
      values: { 'aa.navigator': 'window.navigator' }
    }),

seems to do the trick. This is with rollup 0.41.6 and firebase 3.9.0 / 4.0.0. It would seem that my problems are unrelated to the original issue reported here, which for me seems to work out of the box.

[kleeb]

any news?

[Shyam-Chen]

Try firebase v4.1.

[kleeb]

Thanks @Shyam-Chen !
firebase 4.1.2 solved it
for Ionic / Angular users angularfire2 4.0.0-rc.1

[Rich-Harris]

fixed in 0.46