Fixes lookup for echarts in theme files #35
Conversation
|
Fixes #34 |
| factory({}, root.echarts); | ||
| } | ||
| })(this, function(exports, echarts) { | ||
| })(this || window, function(exports, echarts) { |
There was a problem hiding this comment.
is it possible to find some a long term solution, because once I'll decide to upgrade JS I need to update it again, and I'm 100% sure I won't do it.
There was a problem hiding this comment.
Not following, window should be always there for browsers. Can you give me example where it would fail?
There was a problem hiding this comment.
if I decide to upgrade eCharts library, I need to make the same changes again
There was a problem hiding this comment.
maybe I downloaded the wrong theme files? https://echarts.apache.org/en/download.html
There was a problem hiding this comment.
Theme files look good and I see your point we can't change theme files. The download files seems to be correct. Let me see how import charts load themes, will try to come up with solution that does not depend on changing theme files
|
closed by #37 |
2 participants
In the theme files
thisis passed as root or perhaps the global that might hold the echarts object. However in browser this is undefined and we are actually looking forwindow. This PR adds a fallback but it does not make the change within the actual function to define consumersthis