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

Support extending classes #33

Closed
4 of 6 tasks
mohebifar opened this issue Apr 16, 2015 · 6 comments
Closed
4 of 6 tasks

Support extending classes #33

mohebifar opened this issue Apr 16, 2015 · 6 comments

Comments

@mohebifar
Copy link
Member

mohebifar commented Apr 16, 2015

Edited by @nene:
The initial implementation has been merged in from #183, some things remain to be improved:

  • We're currently overly strict by not detecting inheritance in the following code:

    function Class1() {}
    Class1.prototype = new Class2();

    we only detect the inheritance when the above is followed by constructor assignment:

    Class1.prototype.constructor = Class2;
  • We detect importing util.inherits using require():

    var util = require("util");
    util.inherits(Class, SuperClass);

    but we'll fail with:

    import util from "util";
    util.inherits(Class, SuperClass);
  • Call to superclass constructor must currently be directly inside the body of the constructor, the following will not work:

    function MyClass() {
      if (someCondition) {
        SuperClass.call(this);
      }
    }
  • super() must be called before any references to this. The following code will fail:

    constructor() {
      this.age = 10; // ReferenceError
      super();
    }
  • super() must always be called from subclass constructor. The following code will fail:

    class Foo extends Bar {
      constructor() {
        this.age = 10; // super() not called
      }
    }
  • We should also convert superclass method calls inside other methods. Converting this:

    foo() {
      SuperClass.prototype.foo(this, params);
    }

    into this:

    foo() {
      super.foo(params);
    }
@mohebifar mohebifar changed the title Support classes super and extends Support extending classes Apr 16, 2015
@vivinjoy
Copy link

+1

@vivinjoy
Copy link

@mohebifar , can we add transpiler dependent transforms for class inheritance? Since typescript implementation and coffeescript implementation of class inheritance differs slightly. I have created a transform for extending classes created via typescript transpiler. I could raise that PR.

@apexearth
Copy link
Contributor

Do we want this added to the class transform or as an additional transform to be used in conjunction with the class transform?

@nene
Copy link
Collaborator

nene commented Aug 21, 2016

I think ideally it should be part of the class transform. But perhaps there are some practical considerations for not making it so.

nene added a commit that referenced this issue Sep 16, 2016
nene added a commit that referenced this issue Sep 16, 2016
nene added a commit that referenced this issue Sep 17, 2016
These two branches are 100% the same.

Refs #33
nene added a commit that referenced this issue Sep 17, 2016
nene added a commit that referenced this issue Sep 17, 2016
nene added a commit that referenced this issue Sep 18, 2016
nene added a commit that referenced this issue Sep 18, 2016
Make it obvious which variable we expect to be parent class.

Refs #33
nene added a commit that referenced this issue Sep 18, 2016
nene added a commit that referenced this issue Sep 18, 2016
nene added a commit that referenced this issue Sep 18, 2016
nene added a commit that referenced this issue Sep 18, 2016
nene added a commit that referenced this issue Sep 18, 2016
Just use simple node.type comparison and make the matchers for
the specific require-s more specific.

Refs #33
nene added a commit that referenced this issue Sep 18, 2016
Instead only use this.inheritsNode and let the RequireDetector
return full AST that we must match against - not just identifier
node.

Refs #33
nene added a commit that referenced this issue Sep 18, 2016
nene added a commit that referenced this issue Sep 18, 2016
Instead of hard-coding invocations of the specific detectors.

Refs #33
nene added a commit that referenced this issue Sep 18, 2016
nene added a commit that referenced this issue Sep 18, 2016
As they are no more shared between different transforms.

Refs #33
nene added a commit that referenced this issue Sep 19, 2016
nene added a commit that referenced this issue Sep 29, 2016
@nene
Copy link
Collaborator

nene commented Sep 29, 2016

@apexearth The inheritance feature has now been finally released in 2.4.0. Should have released it sooner, but thought that I'll solve the two remaining small problems. But didn't really found time to tackle them.

@nene
Copy link
Collaborator

nene commented Sep 29, 2016

Closing this old task. Created #186 to handle the two remaining issues.

@nene nene closed this as completed Sep 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants