Polymer / tools

Polymer Tools Monorepo
BSD 3-Clause "New" or "Revised" License
430 stars 200 forks source link

polymer-build breaks the app-storage/app-indexeddb-mirror element #1182

Closed abdonrd closed 5 years ago

abdonrd commented 8 years ago

Description

polymer-build breaks the app-storage/app-indexeddb-mirror element.

Versions & Environment

git clone https://github.com/GDGSpain/gdg.es.git

npm install && bower install

gulp polymer-build

polymer serve build/bundled

Open the localhost:8080/groups page.

Expected Results

screen shot 2016-08-05 at 00 32 33

Achieved with the polymer serve.

Actual Results

screen shot 2016-08-05 at 00 31 23
abdonrd commented 8 years ago

@FredKSchott have been able to review it? Thanks in advance!

FredKSchott commented 8 years ago

@abdonrd sorry I still haven't been fix the problem you're seeing with that element. I'll try to find some time next week but I can't promise I'll be able get to it :(

Have you tried digging into the build command yourself? Try using the --verbose flag & maybe adding some logging of your own, you might be able to spot something that I missed when I first looked into this.

abdonrd commented 8 years ago

@FredKSchott If I add app-indexeddb-mirror-worker.js and common-worker-scope.js in the includeDependencies works in the unbundled version.

"includeDependencies": [
  "bower_components/app-storage/app-indexeddb-mirror/app-indexeddb-mirror-worker.js",
  "bower_components/app-storage/app-indexeddb-mirror/common-worker-scope.js",
  "bower_components/webcomponentsjs/webcomponents-lite.min.js",
  "LICENSE.txt",
  "app.yaml",
  "manifest.json"
]
screen shot 2016-09-21 at 09 47 08

But still it is broken in the bundled version. 😔

PS: No errors with --verbose flag.

FredKSchott commented 8 years ago

Fixed by https://github.com/GDGSpain/gdg.es/pull/34 We'll be fixing this more generally by outputting these errors. Tracking here: https://github.com/Polymer/polymer-build/issues/52

abdonrd commented 8 years ago

Comment at https://github.com/GDGSpain/gdg.es/pull/34#issuecomment-249332748

FredKSchott commented 8 years ago

Oof, okay reopening

abdonrd commented 8 years ago

@FredKSchott the new polymer-analyzer will solve this problem? Thanks in advance!

FredKSchott commented 7 years ago

@abdonrd the new analyzer was added in v0.5.0, can you download the latest version from npm and see if that fixes it? Fingers crossed for you...

FredKSchott commented 7 years ago

Otherwise, please update your steps to reproduce (currently getting Error: File not found with singular glob: /Users/fschott/Code/gdges/app.yaml) and I'll take a look.

abdonrd commented 7 years ago

Thanks @FredKSchott! 🙏

I has updated develop branch fixing this error.

abdonrd commented 7 years ago

Right now, the behaviors (with your https://github.com/GDGSpain/gdg.es/pull/34) break the build:

screen shot 2016-11-02 at 02 20 31
FredKSchott commented 7 years ago

You are seeing that output on my #34 branch? Hmm… That's strange because that's exactly what that branch is supposed to fix.

Will take a look first thing tomorrow morning On Tue, Nov 1, 2016 at 6:21 PM Abdón Rodríguez Davila < notifications@github.com> wrote:

Right now, the behaviors (with your GDGSpain/gdg.es#34 https://github.com/GDGSpain/gdg.es/pull/34) break the build:

[image: screen shot 2016-11-02 at 02 20 31] https://cloud.githubusercontent.com/assets/1007051/19913451/fe5df6b8-a0a2-11e6-9f9e-0f636feadfa1.png

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/Polymer/polymer-build/issues/22#issuecomment-257746627, or mute the thread https://github.com/notifications/unsubscribe-auth/AAl-kxK6aX8NzGEfMQ8vQGoKDnHAU2keks5q5-WAgaJpZM4Jd4vu .

abdonrd commented 7 years ago

Yes, I have your branch with extra changes (https://github.com/GDGSpain/gdg.es/pull/43).

FredKSchott commented 7 years ago

Okay I dug in a bit today, and uglify is stripping out comments in your build process. Because of this the analyzer isn't able to find the @polymerBehavior tag that it needs to parse.

So the quick fix is to remove JS minification from the build process for now. My copy of gdg.es builds when that step is removed.

I've tried adding uglify back in with the {preserveComments: 'all'} option, but it's still having trouble with extended behaviors (* & *Impl). I'll keep looking into it, it looks like it might be a problem in the new analyzer.

abdonrd commented 7 years ago

Thanks @FredKSchott!

Anyway, even disabling the uglify (the npm run build works for me too), the original problem persists. You can check it in https://github.com/GDGSpain/gdg.es/pull/43/files

Chrome:

screen shot 2016-11-02 at 18 32 40

Safari:

screen shot 2016-11-02 at 18 33 56
FredKSchott commented 7 years ago

So I've looked into this again and I don't think there's anything wrong in build/analyzer now. This might just be a problem between firebase/app-storage elements and bundling.

Can you reduce this down to the smallest reproducible test case? It would be great if we could find out what exactly is breaking on the groups page without the complexity of the rest of your app. That will help us track down and fix this issue as quickly as possible.

abdonrd commented 7 years ago

@FredKSchott tell me if it works for you: abdonrd/polymer-build-app-storage-issue :)

midesweb commented 7 years ago

for me worked with "includeDependencies" as @abdonrd says, both bundled and unbundled builds

motss commented 7 years ago

@abdonrd @FredKSchott @midesweb This is how I resolve this issue - to include the .js files inside app-storage/app-indexeddb-mirror for both bundled and unbundled.

  "sourceGlobs": [
   "src/**/*",
   "images/**/*",
   "bower.json",
   "bower_components/app-storage/app-indexeddb-mirror/*.js"
  ],
abdonrd commented 7 years ago

@midesweb @motss but it doesn't work in Safari.

And we shouldn't have to add that in the manifest.json. It should be polymer-build work to find it.

n1ywb commented 7 years ago

I think this is related https://github.com/PolymerElements/app-storage/issues/78 and https://github.com/notwaldorf/mojibrag/issues/49

abdonrd commented 7 years ago

@FredKSchott have you been able to review the https://github.com/Polymer/polymer-build/issues/22#issuecomment-260749112?

fairsayan commented 7 years ago

I tried both @motss and @n1ywb solutions with bundled option: it works only on Chrome. With Firefox and Safari I got errors and basically I never obtain the "App IndexedDB Client connected!" message :-( Here Safari error:

screen shot 2017-02-06 at 19 15 44

Looking forward to have this feature fixed.

fairsayan commented 7 years ago

PS: I also tried a mix between the 2 solutions, since I realized ./src/common-worker-scope.js doesn't exists, but I fixed it just on Safari (and Chrome 56.0 on my Mac as I wrote before). Here the error on Chrome 55.0 on Android 4.4.2 for the bundled version:

screen shot 2017-02-06 at 21 22 14

it says the file doesn't exists even if I can get it using wget https://by-wink-timeshift.firebaseapp.com/bower_components/app-storage/app-indexeddb-mirror/common-worker-scope.js?https://by-wink-timeshift.firebaseapp.com/bower_components/app-storage/app-indexeddb-mirror/app-indexeddb-mirror-worker.js

Chrome 55.0 on Android 4.4.2 for the unbundled version works fine with my last dirty fixes.

abdonrd commented 7 years ago

@FredKSchott any news? :)

kownacki commented 7 years ago

I got this problem because of my service worker. I'm pretty sure you also have one and it also is the cause for you. I solved this problem by adding this to sw-precache-config.js:

ignoreUrlParametersMatching: [/^<your origin here>/]

So for example for @abdonrd it would be:

ignoreUrlParametersMatching: [/^https:\/\/gdg-es-develop\.firebaseapp\.com/]

Of course, as you already found out, it's also required to add this to polymer.json:

"extraDependencies": [
  "bower_components/app-storage/app-indexeddb-mirror/app-indexeddb-mirror-worker.js",
  "bower_components/app-storage/app-indexeddb-mirror/common-worker-scope.js"
]

Explanation

As it's written here

sw-precache finds matching cache entries by doing a comparison with the full request URL

So https://gdg-es-develop.firebaseapp.com/bower_components/app-storage/app-indexeddb-mirror/common-worker-scope.js?https://gdg-es-develop.firebaseapp.com/bower_components/app-storage/app-indexeddb-mirror/app-indexeddb-mirror-worker.js because of this crazy parameter it couln't find this URL among the precached and so it returned 404.

For me it should be at least noted in the app-storage documentation like it's noted about extraDependencies link

Or it should be redesigned to avoid problems like this at all.

abdonrd commented 7 years ago

Thanks @kownacki!

I update with some info from https://github.com/GDGSpain/gdg.es/issues/32#issuecomment-293882241:

After:

  • Update app-storage to v1.0.0
  • Update polymer-build to v1.0.0
  • Add app-storage extraDependencies in the polymer.json

Works in Chrome and Firefox! 🎉

But it still does not work on Safari.

screen shot 2017-06-14 at 2 51 38 pm 1

kownacki commented 7 years ago

@abdonrd It is enough to make it working in desktop browsers where Shared Workers are available. But app-storage on android switches to Web Workers which causes more problems which can be solved with ignoreUrlParametersMatching.

I hope it helps :)

abdonrd commented 7 years ago

@kownacki I has tried what you said at https://github.com/Polymer/polymer-build/issues/22#issuecomment-307191630, but in Safari (macOS) continues to fail.

screen shot 2017-06-14 at 2 39 03 pm

Btw, Safari doesn't have SW support. Then the sw-precache-config.js doesn't affect.

kownacki commented 7 years ago

@abdonrd If there is no Service Worker then I see no other option than that this this file is simply not available on the server, thus 404. You noticed that it happens only on the bundled version. I don't bundle my code with polymer-build. There must be still something wrong with the bundle process.

abdonrd commented 7 years ago

@kownacki yes, before the build it works:

screen shot 2017-06-14 at 3 38 48 pm
andreataglia commented 6 years ago

@kownacki your solution works for me! Thanks a lot!

abdonrd commented 5 years ago

Closing and old issue.