Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use "this" in static methods #2532

Closed
joeyparrish opened this issue Apr 27, 2020 · 2 comments
Closed

Use "this" in static methods #2532

joeyparrish opened this issue Apr 27, 2020 · 2 comments
Labels
priority: P3 Useful but not urgent status: archived Archived and locked; will not be updated type: code health A code health issue

Comments

@joeyparrish
Copy link
Member

To solve #2528, we need to upgrade the compiler. A side-benefit of the upgrade is that we will finally be able to use this in static methods. The use of this in static methods has long been part of ES6 classes, but the compiler version we used did not support it.

We should clean up our many, many self-aliases in static methods. For example, in lib/util/string_utils.js, we currently have this:

  static fromBytesAutoDetect(data) {
    const StringUtils = shaka.util.StringUtils;
    // ...
      return StringUtils.fromUTF8(uint8);

The alias would be removed and the call site would use this instead:

  static fromBytesAutoDetect(data) {
    // ...
      return this.fromUTF8(uint8);
@joeyparrish joeyparrish added the type: code health A code health issue label Apr 27, 2020
@joeyparrish joeyparrish added this to the Backlog milestone Apr 27, 2020
shaka-bot pushed a commit that referenced this issue Apr 30, 2020
We used to alias static utility methods by assigning the method itself
to a local variable.  This is not allowed by the new Closure Compiler
release that we are adopting, and for good reason.

The old compiler did not understand the use of "this" in static
methods, but the new one does.  And it turns out that when we start
using "this" in static methods (see #2532), aliasing the method itself
can break everything.

When you refer to "this" in a static method, it refers to the class.
This is really useful in utility classes that have private static
methods they use to perform common tasks.  However, just as it ruins
the value of "this" to alias an instance method, the same is true of
an ES6 class's static method.

The fix is to always alias the class instead of the method.  The new
compiler will simply not let us get away with the old way any more, so
regressions after this are unlikely.

Issue #2528 (compiler upgrade)
Issue #2532 (static "this")

Change-Id: Id800d466e639c7cbcf4aa6fbb05114c772a2229f
@joeyparrish joeyparrish added the priority: P3 Useful but not urgent label Sep 29, 2021
joeyparrish pushed a commit that referenced this issue Feb 28, 2024
@avelad
Copy link
Member

avelad commented Apr 22, 2024

Since our plan is to migrate to TypeScript, @joeyparrish, are you okay if we close this issue?

@avelad avelad added the status: waiting on response Waiting on a response from the reporter(s) of the issue label Apr 22, 2024
@joeyparrish
Copy link
Member Author

Yes, fine with me.

@shaka-bot shaka-bot removed the status: waiting on response Waiting on a response from the reporter(s) of the issue label Apr 22, 2024
@avelad avelad removed this from the Backlog milestone Apr 30, 2024
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Jun 21, 2024
@shaka-project shaka-project locked as resolved and limited conversation to collaborators Jun 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
priority: P3 Useful but not urgent status: archived Archived and locked; will not be updated type: code health A code health issue
Projects
None yet
Development

No branches or pull requests

3 participants