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

More js-compatible classes and support for decorators #269

Closed
ark120202 opened this issue Oct 25, 2018 · 7 comments
Closed

More js-compatible classes and support for decorators #269

ark120202 opened this issue Oct 25, 2018 · 7 comments

Comments

@ark120202
Copy link
Contributor

To implement decorator support classes should be constructed in the runtime

class A {
  private a: number;
  private initializedField = Math.random();
  constructor(a: number) {
    this.a = a;
  }

  @DecoratedMethod
  foo() {
    return this.a + 1;
  }

  get bar() {
    return this.a * 2;
  }

  static staticMethod() {}
}
local A = ___TS_CreateClass({
  decorators = {Decorated},
  -- May be called with this = nil when used A() form
  constructor = function(this, a)
    this.a = a
  end,
  fields = {
    {
      kind = "field",
      key = "a",
      value = function() end
    },
    {
      kind = "field",
      key = "initializedField",
      value = function() return math:random() end
    },
    {
      kind = "method",
      decorators = {DecoratedMethod},
      key = "foo",
      value = function(this) return this.a + 1 end
    },
    {
      kind = "get",
      key = "bar",
      value = function(this) return this.a * 2 end
    },
    {
      kind = "method",
      static = true,
      key = "staticMethod",
      value = function() end
    }
  }
})

For more details and reasoning of using such a complex way to declare classes check out:

Doing makes generated code further from what you'd write in Lua, but makes it match JS behavior more and gives a chance to solve #229 and #256.

@Perryvw
Copy link
Member

Perryvw commented Oct 25, 2018

I think for now this is not something we want to consider for this project (yet). Like discussed in other issues we do not necessarily want to recreate exact JavaScript behavior. I am not convinced yet this added complexity is worth it.

@ark120202
Copy link
Contributor Author

I don't think there's any simple way to implement decorators, even if some of it's specifics would be dropped. So it's either some complicated way constructing classes or not supporting decorators at all.

@andreiradu
Copy link
Contributor

I think you're overcomplicating this. Here's your example bellow. Note that I completely ignore the property descriptor from the signature - I don't think the complexity and the runtime cost of having things like non-enumerable properties is really worth it. The behavior bellow seems to be consistent to how typescript currently handles decorators.

function DecoratedMethod(target: any, propertyKey: string) {
    const func = target[propertyKey];
    target[propertyKey] = function wrapper(...args) {
        return func(...args);
    }
};
class A {
    private a: number;
    private initializedField = Math.random();
    constructor(a: number) {
      this.a = a;
    }
  
    @DecoratedMethod
    foo() {
      return this.a + 1;
    }
  
    static staticMethod() {}
  }

now transpiles to

function DecoratedMethod(target: any, propertyKey: string) {
    const func = target[propertyKey];
    target[propertyKey] = function wrapper(...args) {
        return func(...args);
    }
};
class A {
    private a: number;
    private initializedField = Math.random();
    constructor(a: number) {
      this.a = a;
    }
  
    @DecoratedMethod
    foo() {
      return this.a + 1;
    }
  
    static staticMethod() {}
  }

We just need it to transpile to


-- Lua Library Imports
function DecoratedMethod(target,propertyKey)
    local func = target[propertyKey];
    target[propertyKey] = function(args)
        return func(table.unpack(args))
    end
;
end

A = A or {}
A.__index = A
function A.new(construct, ...)
    local self = setmetatable({}, A)
    self.initializedField = math.random()
    if construct and A.constructor then A.constructor(self, ...) end
    return self
end
function A.constructor(self,a)
    self.a = a;
end
function A.foo(self)
    return self.a+1
end
DecoratedMethod(A, "foo"); -- new
function A.staticMethod(self)
end

@ark120202
Copy link
Contributor Author

The behavior bellow seems to be consistent to how typescript currently handles decorators.

TypeScript (and most of decorator-related things in JS now) implements a stage-0 decorators proposal. After that it changed a lot. Of course these decorators may be implemented, but stage-2 decorators may progress to stage-3, so they would be implemented in TypeScript (now there's only Babel support). No idea how TS team would handle migration, but in the end someday they will have to remove legacy proposal anyway.

@andreiradu
Copy link
Contributor

Honestly, I have no idea how typescript will implement the stage-2 decorator proposal(and I don't fully understand the implications of it). It's not only a matter of migration(which would obviously be a pain),
it's also a matter of how they interact with the type system. You can read this issue.
From my point of view there is great value in decorators, and it would be a shame to put the perfect ahead of the good.

@ark120202
Copy link
Contributor Author

Sure, I'm not saying that old decorators shouldn't be implemented. I assume that typescript would use different option for new proposal, so both proposals may be supported independently.

@ark120202
Copy link
Contributor Author

Closing because of recent changes in the proposal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants