Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Does't work when bundled with --js-minify #197

Open
3 of 9 tasks
tpluscode opened this issue Apr 10, 2017 · 8 comments
Open
3 of 9 tasks

Does't work when bundled with --js-minify #197

tpluscode opened this issue Apr 10, 2017 · 8 comments

Comments

@tpluscode
Copy link

Description

When I use polymerfire and try to polymer build --js-minify authentication breaks

Expected outcome

Should work just like in unbundled form

Actual outcome

When already signed-in, the elements don't authenticate. We use email+password authentication and when we try Chrome console shows

Uncaught TypeError: Cannot read property 'signInWithEmailAndPassword' of undefined

Similar with Firefox

Steps to reproduce

You can reproduce by checking out this branch: https://github.com/PGS-dev/framapp-polymer/tree/lazy-imports and running

polymer build --js-minify
polymer serve --open

Browsers Affected

  • Chrome
  • Firefox
  • Safari 9
  • Safari 8
  • Safari 7
  • Edge
  • IE 11
  • IE 10
  • Opera
@tjmonsi
Copy link
Collaborator

tjmonsi commented Apr 11, 2017

Please have the branch in its minimum state:
Problems encountered while reproducing:

  • when running the build, polymer.json seeks app.yaml. It doesn't see the app.yaml in the repo because it is part of the .gitignore file.

Now on to the real problem.
Bundling the JS returns fine. Both firebase-app and scripts are working. The problem is that firebase-app is not initialized for some reason.
image
Even if you have put in your firebase config data from resourcesBehavior, it somehow doesn't initialize firebase-app. I do know that js-minifier has affected the way firebase-app initializes but it doesn't really break the polymerfire system.

I do have found a solution:
In your resources-behavior.html. add this in your authData:

authData: {
          type: Object,
          value: {
            authDomain: "project-5613440220430148247.firebaseapp.com",
            databaseUrl: "https://project-5613440220430148247.firebaseio.com",
            apiKey: "AIzaSyBKP4cOP508h0JLKmjFvzJooO0MqV8l4fU",
            storageBucket: "1",
            messagingSenderId: "1"
          }
        }

and then in your fp-login-component.html, add this in your firebase-app

<firebase-app
        auth-domain="[[authData.authDomain]]"
        database-url="[[authData.databaseUrl]]"
        api-key="[[authData.apiKey]]"
        messaging-sender-id="[[authData.messagingSenderId]]"
        storage-bucket="[[authData.storageBucket]]">

That way it puts in the messaging-sender-id and storage-bucket. Now don't worry, it will not initialize the messaging and storage of firebase because you didn't import the firebase-messaging-script and firebase-storage-script. But apparently, after minifying the js, Polymer now doesn't computes the initialization of firebase app, as seen here:

/**
           * The Firebase app object constructed from the other fields of
           * this element.
           */
          app: {
            type: Object,
            notify: true,
            computed: '__computeApp(name, apiKey, authDomain, databaseUrl, storageBucket, messagingSenderId)'
          }

The thing with Polymer is it doesn't run the computed attribute of the app if at least one of the attributes are undefined. Given that the original value of name is just '' and you have apiKey, authDomain, and databaseUrl, that leaves storageBucket and messagingSenderId as null... and even though this should work, I think the js minifier does something and makes these things undefined, rendering the firebase app not to initialize.

So for your fix:
Just add the messagingSenderId and storageBucket values for now and it should work.

As to why your cards are not showing up, that I didn't investigate :)

@tjmonsi
Copy link
Collaborator

tjmonsi commented Apr 11, 2017

I also know now why the cards from products are not showing up. It's because the category is null. When I clicked on category, it works... it is again because of this:

products: {
            type: Array,
            notify: true,
            computed: '_computeProducts(category, promotedOnly, _recomputeHack)',
          },

Thus, when category is null... it doesn't work.

I think if I am not mistaken, when polymer build minifies javascript through --js-minifier, it minifies the Polymer library as well, and I think when observing changes in properties, it just makes undefined and null equal to each other. Thus, it doesn't run computed and observer functions when at least one of the parameters are undefined and null because when minified, undefined == null. But that is just a hunch :)

@tpluscode
Copy link
Author

tpluscode commented Apr 11, 2017

Hi. Thank you so much for taking your time to investigate thoroughly. Unfortunately though your suggestions did not help me. I will try to compile a minimal reproduction for you to test against.

Speaking of which, are you sure you were running on the lazy-imports branch? That branch no longer has the app.yaml present in polymer.json...

Anyways, I'm still getting this error stack whever I try to log in..

Uncaught TypeError: Cannot read property 'signInWithEmailAndPassword' of undefined
    at HTMLElement.signInWithEmailAndPassword (firebase-auth.html:1)
    at HTMLElement.handleClick (fp-login-component.html:1)
    at HTMLElement.S (framapp-polymer-app.html:1)
    at HTMLElement.fire (framapp-polymer-app.html:1)
    at Object.fire (framapp-polymer-app.html:1)
    at Object.forward (framapp-polymer-app.html:1)
    at Object.click (framapp-polymer-app.html:1)
    at HTMLElement.handleNative (framapp-polymer-app.html:1)

@tjmonsi
Copy link
Collaborator

tjmonsi commented Apr 11, 2017

Ohhh. I was running on master. But when I looked at the fc-login-component.html from both branches, it is the same. Can you push it to your repo so I can pull it up to mine and see why my suggestions are not working?

Either that or you need to use CTRL + SHIFT + R to do refresh without using the cache. And don't forget to rerun the compiler (I had that problem just now when I was first testing your app, only to find out I didn't compile it hahaha)

@tpluscode
Copy link
Author

Sure, I just pushed latest changes.

In fact something changed and now I'm seeing this same error even in unbundled build. Hell, even in simple polymer serve :(. No cache, nothing. When Polymer hits _computeAuth in firebase-auth, this.app.auth is undefined. This fails silently and so this.auth remains undefined too

@tpluscode
Copy link
Author

tpluscode commented Apr 11, 2017

OIC! It started ocurring in unminified from when I added firebase-* elements to fragments in polymer.json.

When minified, I indeed had to additionally apply your solution to overcome the compute problem. Guess I should report it somewhere. Core Polymer repo maybe?

And what about lazy-loading? Polymerfire adds about 100KB to the minified bundle. I definitely would prefer it to stay out. Would you like me to prepare a separate example for that issue alone?

@tjmonsi
Copy link
Collaborator

tjmonsi commented Apr 12, 2017

Well polymer --js-minify works better I think for Polymer 2.0 versions. And computed properties work differently in Polymer 2.0 from 1.0.

@tjmonsi
Copy link
Collaborator

tjmonsi commented Apr 20, 2017

@tpluscode I just thought of an idea that might help you on lazy-loading.

What I usually do before was that i just add polymerfire up on the core because I really need it to be part of the first content, especially because the content that's needed to be up on screen is from firebase.

Then I remember, firebase content, especially the ones that doesn't need any form of authentication (like blog articles, or other things), can be retrieved using ajax calls. So just use iron-ajax instead for critical information or first content, then just lazy load firebase after the first content has been painted.

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

No branches or pull requests

2 participants