Esri / ember-cli-cedar

Ember addon for Esri's Cedar charting library
https://esri.github.io/ember-cli-cedar/#/charts/bar
Apache License 2.0
3 stars 2 forks source link

fix broken builds by bundling arcgis-rest-js libs in vendor.js #90

Closed tomwayson closed 6 years ago

tomwayson commented 6 years ago

should not have published beta.3 w/ "@esri/cedar": "^1.0.0-beta.3" dependency, b/c @esri/cedar@1.0.0-beta.4 introduced a breaking change by treating the arcgis-rest-js deps as external.

This fixes that by explicitly bumping to @esri/cedar@1.0.0-beta.4 and including the arcgis-rest-js files vendor.js in preparation for this libraries own beta.4 release.

jgravois commented 6 years ago

👍 i approve this pull request. - @geogangster

tomwayson commented 6 years ago

cc @jgravois it looks like it's not a problem to have an Ember addon import the arcgis-rest-js libs into vendor.js. See https://github.com/Esri/ember-cli-cedar/pull/90/files#diff-168726dbe96b3ce427e7fedce31bb0bc for the changes that were needed.

One thing that I did not do was make a shim b/c cedar wraps these functions so there's no need to do something like import { queryFeatures } from '@esri/arcgis-rest-feature-service';. I bet the shim is where you were having trouble before, and I can see how that's going to be a bit tricky w/ multiple packages sharing the same global, etc). Strictly speaking, a shim is not needed. The Ember app code can just use the arcgisRest global directly. I'd suggest starting w/o the shim to get things working, then we can add the shim later.

One other thing, when adding these in other Ember addons, I think it will be important to use the same destDir as I've used for each package. I think this is how Ember decides if it has already imported that file into the vendor.js file, so, for example, always using @esri/arcgis-rest-request for that package will prevent it from being imported twice. @mjuniper or @dbouwman may know better than I whether or not that is true or just an old developer's tale.