mathjax / MathJax-src

MathJax source code for version 3 and beyond
https://www.mathjax.org/
Apache License 2.0
2.05k stars 205 forks source link

Corrects static method calls #1087

Closed zorkow closed 5 months ago

zorkow commented 5 months ago

Rewrites method calls this.makeProcessor to Configuration.makeProcessor as makeProcessor is actually a static method. It was probably pure luck that this never failed and terser did not uniquely renames static methods.

dpvc commented 5 months ago

I don't understand the need for this. The _create() method where these calls are made is also a static method, so this will be the Configuration object already. I don' think terser can rename the static methods (any more than it can rename the instance methods), and if it did, it would need to change these references as well.

It is actually valuable to keep these as this, so that if Configuration is subclassed and overrides the makePorcessor() method, for example, you won't have to also override _create() to call the subclass instead of Configuration. I have run into this difficulty in the past when subclassing objects that call their class name explicitly rather than using this in static methods and this.constructor in instance methods. So I'd prefer that these stay as this.

dpvc commented 5 months ago

There also are other places where this is used in static methods, so if these need to be changed for some reason, shouldn't those others as well?

zorkow commented 5 months ago

As discussed f2f, this came out of experiments with tsickle and closure-compiler, which do not like the references to static methods via this nor our way of using this.constructor.

Currently, we are not going for that. PR closed.