nickcam / AnimatedEnvironmentLayer

A custom arcgis js api layer to display GRIB data formatted as json as animated particles on a canvas
https://animatedenvironmentlayer.azurewebsites.net/index.html
44 stars 10 forks source link

Dojo declare and API 4.19? #8

Closed hlam94 closed 1 year ago

hlam94 commented 3 years ago

Hi! I'm trying to learn more about developing with ArcGIS (and web development in general) and want to ask about the declaration for the AnimatedEnvironmentLayer class in animatedEnvironmentLayer.ts (line 412):

export class AnimatedEnvironmentLayer extends asd.declared(GraphicsLayer) {

Why can this not simply be:

export class AnimatedEnvironmentLayer extends GraphicsLayer {

?

I am trying to use the class in an app running ArcGIS js API 4.19 and followed advice here- https://developers.arcgis.com/javascript/latest/4.16/#previous-typescript-syntax -to replace the deprecated declared() method, but making the suggested change raises some nonspecific error when creating the layer object.

Sorry if I'm troubling you over old code. Any advice is appreciated!

nickcam commented 3 years ago

Hi @hlam94 - I haven't tested this on v4.19 yet. Apparently esri have removed dojo dependencies in that release, so their module / class system has changed. I can see this causing all sort of problems without some refactoring.

What you've done to remove the declared call is correct, that was an old style way to extend their Accessor classes. It's probably not that which is the problem though, but instead major changes to their api.

If I get a chance I'll look at upgrading this, but I don't use it anymore personally and haven't looked at the arcgis js api for a while as well - I can't guarantee I'll get around to it. If you work something out feel free to submit a PR.

Brandonpellis commented 3 years ago

We did 2 things to get vectors to show up in 4.20

1.) We found no documented changes to the BaseLayerView2D object, but found that the extended attach method was not being called by ESRI as it had been in previous API versions. It is unknown whether the issue is with the way the ESRI object is behaving or that this object is not able to extended the method properly. Once we confirmed that this.attach() was no longer being triggered the same way post 4.15, the issue was resolved by manually calling this.attach() when AnimatedEnvironmentLayerView2D was called. We tried to call the attach method in predraw because BaseLayerView2D is reported to call its attach method just before the layer draws but doing so in this code would destroy the data because the windy object is overwritten and the data is destroyed. I do not think that this is a proper fix, but it is a slap job for now. Maybe a bigger brain has a proper way to resolve the issue.

function AnimatedEnvironmentLayerView2D(props) {
            var _this = _super.call(this) || this;
            _this.view = props.view;
            _this.layer = props.layer;
            _this.view.on("resize", function () {
                if (!_this.context)
                    return;
                // resize the canvas
                _this.context.canvas.width = _this.view.width;
                _this.context.canvas.height = _this.view.height;
            });
            watchUtils.watch(_this.layer, "visible", function (nv, olv, pn, ta) {
                if (!nv) {
                    _this.clear();
                }
                else {
                    _this.prepDraw();
                }
            });   

            this.attach();

            return _this;
        }

2.) 4.15 release notes suggested that .otherwise() will no longer be supported as they have moved to native promises rather than dojo promises. (https://developers.arcgis.com/javascript/latest/4.15/#return-native-promises)

We experienced fails here that were likely unrelated to this change, but we made it anyway and it seems to work without issue.

 esriRequest(this.url, {
                    responseType: "json"
                })
                    .then(function (response) {
                    _this.dataFetchRequired = false;
                    _this.doDraw(response.data); // all sorted draw now.
                    _this.dataLoading = false;
                })
                    .catch(function (err) {
                    console.error("Error occurred retrieving data. " + err);
                    _this.dataLoading = false;
                    _this.isErrored = true;
                });
nickcam commented 1 year ago

I merged a PR recently that provided support for v4.21. So closing this.

11 PR to update to 4.21