DraggableLinesHandler is missing methods when compiled to ES5 (with CRA)

[lehnerpat]

Hey there,
thanks for this neat library, I was super happy to find this functionality as a plugin for Leaflet :)

I've been using DraggableLines in a React project, and thought all was fine, but recently I noticed that in the production build it broke at a place where I use DraggableLinesHandler.enableForLayer() and disableForLayer() to switch it on and off.
I don't expect you to fully support all scenarios, and I'm not even sure if this is a problem your code or somewhere else, but I thought I'd report this just in case someone else has a similar issue.

---

First of all, some relevant package versions (taken from my yarn.lock):

• Leaflet: 1.8.0
• leaflet-draggable-lines: 1.1.0
• react: 18.1.0
• react-scripts (i.e. create-react-app): 5.0.1
• webpack: 5.69.1

The related issue in my project is: lehnerpat/train-ride-maps#20

Now, for some more details:

• I'm using a standard CRA (=create-react-app) setup with Typescript.
• With the default settings I'm using, CRA uses babel and webpack to compile and bundle my sources.
• Using browserslist, it decides how to compile the app, i.e. what language level to target and what polyfills to include. I'm using the default setting:

  "browserslist": {
      "production": [">0.2%", "not dead", "not op_mini all"],
      "development": ["last 1 chrome version", "last 1 firefox version", "last 1 safari version"]
  }

  • This currently results in "development" builds being compiled to ES6, while "production" builds are compiled to ES5.
• In the development build, DraggableLines works completely fine.
• In the production build, when trying to call the enableForLayer() method on a DraggableLines instance, I get the error:

  Uncaught TypeError: t.draggableLines.enableForLayer is not a function
  

  • and similarly for disableForLayer()
• Investigating in the browser inspector (Chrome DevTools), I found out that the DraggableLines instance (t.draggableLines) doesn't have any of the methods implemented in DraggableLinesHandler; it only has the method of Leaflet's stock Handler.
• From what I can tell, the (ES5-compiled) prototype of DraggableLinesHandler has the methods, but when constructing the object, something goes wrong with the prototype chain, and the instance's prototype is set to Handler, not DraggableLinesHandler.

I'm continuing to investigate this, partly because DraggableLines really fits my use case and I'd prefer not to implement this myself, and partly because I know what is going wrong here exactly.

One hypothesis I have is that mixing the ES6/TS class-based inheritance you use in DraggableLinesHandler with Leaflet's custom-built class system somehow confuses the Babel transpiler or something.
I'm planning to test this by adding another simple subclass of Handler in my own code, and see if it shows the same behavior, and then possibly also trying to change DraggableLinesHandler to use L.Class.extend() instead of ES6 classes.

If you have any ideas or insights, I'd be happy to hear them :)
I'll report here if I find anything significant.

[cdauth]

I am using DraggableLinesHandler in production and don't have this problem. I'm also building using webpack, so I wouldn't expect my setup to be super different from yours.

I could imagine that maybe the extension methods get tree-shaken away. Could you also try if it works with [email protected]? The build setup was different there, maybe the change has broken something.

[lehnerpat]

Thanks for the quick feedback! That's a good idea, I'll also try that.

Meanwhile, I've managed to narrow the issue down further:

The following class works, i.e. calling "myMethod" works even in production:

class MyHandler extends Handler {
  constructor(map: LeafletMap) {
    super(map);
    console.debug("MyHandler");
  }

  myMethod() {
    console.debug("myMethod");
  }
}

However, when I add

Object.assign(MyHandler.prototype, Evented.prototype);

the method "disappears" in production, just like with DraggableLines.

Tree-shaking was one of my suspects as well, maybe overwriting stuff on the prototype makes webpack/babel lose track of what methods are used and tree-shake too much 🤔

[lehnerpat]

On second thought, tree-shaking is probably not the (sole) culprit -- the method is there in the prototype itself, but when instantiating the object it's not visible in that object's prototype 🤔

Edit: to illustrate:

The (augmented) prototype itself:

But after instantiating:

[lehnerpat]

So, I've been diving deeper into the compiled code, and I think I've now figured out what exactly goes wrong (or rather why the Object.assign makes it go wrong), though I'm not sure why it works for you.
I don't know how much this will help in fixing the problem, but for my own memory and peace of mind I'll record my findings here for now.

---

• Here we see the compiled constructor of MyHandler. The class definition is minified as Af, but calling the ctor (on line 29537 in my previous post) puts you into n(t) (line 29467).
• Calling Ko (1) just makes sure that it's called with the new operator and not as a plain function.
• The call to e (2), indirected via call, is the compiled version of the super(map) call in the original TS, as best I can tell. The this on this line is our newly constructor instance of MyHandler.
• We step into e in the next frame.

---

• in this function, fi (1) seems to be a an alias for the builtin function getPrototype, which just returns an object's prototype.
• the this (2) here is still our newly constructor instance of MyHandler, its prototype has our myMethod
• using this's prototype's ctor to construct a new instance (3) (intended to be the super instance, I think?) seems to make sense
• However, this is where it breaks down. Let's take a look at Af (MyHandler) and its constructor.

---

• At the bottom, we see that Af.prototype has our myMethod.
• However, Af.prototype.constructor.prototype (which is used as fi(this).constructor above) doesn't have it.
• My explanation for this is: when we Object.assign(MyHandler.prototype, Evented.prototype), we also overwrite MyHandler.constructor with Evented.constructor, and Evented.constructor only refers to Evented's own prototype, not to MyHandlers's (augmented) prototype.

So for some reason, the prototype augmentation seems to fall down when compiling to ES5.
I still don't understand why it works fine when compiling to ES6.

---

This is all I have time for today, next time I'll try compiling with DraggableLines v1.0.11 as you suggested :)

[cdauth]

That looks quite complex, I would have to dig into the code to really understand what's going on.

But it somehow seems like the ES5 transpiler is wrapping the constructor in another function, so we are assigning the methods to the prototype of the wrapper, which does not affect the created objects.

I guess that Object.assign() is just not a very good approach to achieve multiple inheritance. If I find time in the next days, I will have a look and try to find a better approach.

[lehnerpat]

Sure thing, there's no rush for this :)
I also definitely don't want to break DraggableLines in another way by "fixing" this.

One thing that might be interesting: Leaflet's offers a L.Class.include() method to add "mixins" to a class. I wonder if this could help here.

If you put together any prototype of an alternative approach, I'd be happy to help test it :)
I'll let you know if I find out anything more.

[cdauth]

I have released a new version where I used a different multi-inheritance mechanism. Could you check if that fixes the issue for you?

[cdauth]

Feel free to reopen this if the issue persists.