PredixDev / predix-webapp-starter

A web application starter kit built on Polymer, Web Components, NodeJS and Predix UI Components
https://predix-webapp-starter.run.aws-usw02-pr.ice.predix.io
Other
61 stars 75 forks source link

Px worker URLs must be declared before main component is loaded? #18

Closed akshayeshenoi closed 6 years ago

akshayeshenoi commented 6 years ago

The declaration of global variable Px.vis.workerUrl possibly doesn't take effect since it's done inside the onMainElementLoaded() method, which is executed after the main element is loaded. The in-lining of the px-vis element inside the main element causes the px-vis-worker.js file to be imported from the wrong location.

gstroup commented 6 years ago

Are you seeing errors, like 404 errors loading the px-vis-worker.js file? I don't see any issues...

akshayeshenoi commented 6 years ago

Yes. Happens on my dist version.

akshayeshenoi commented 6 years ago

Turns out gulp vulcanizes px-vis-scheduler.html for me which puts it in the main element's file. Any clue why that might happen?

gstroup commented 6 years ago

Are you importing the px-vis-scheduler.html file with an html import, like <link rel="import ... >? If so, then the Polymer CLI will include it in one of the bundled files.

akshayeshenoi commented 6 years ago

px-vis-behavior-d3.html depends on px-vis-scheduler.html, and a lot of other components in turn depend on the former. So that probably causes polymer-build to vulcanize px-vis-scheduler.html.

I tried manually importing px-vis-scheduler in rmd-view.html (from master) and the dist version throws this error on run-time: image

Moving these lines out of the function fixes the issue, which declares the variable before the vulcanized script is executed.

gstroup commented 6 years ago

Interesting. I have not seen the errors that you show here, using the current webapp-starter. But what you're saying makes sense. Moving those global variables outside of the onMainElementLoaded() function makes sense. I'll make that change and do some tests. Should be included in the next release. Thanks!

gstroup commented 6 years ago

I'm going to keep this open for tracking until the fix is released. Thanks.

gstroup commented 6 years ago

Fixed in v2.0.5.