Skip to content

Class method autocompletions not working properly on class with mixins #26275

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

Open
mjbvz opened this issue Aug 7, 2018 · 5 comments
Open

Class method autocompletions not working properly on class with mixins #26275

mjbvz opened this issue Aug 7, 2018 · 5 comments
Labels
Bug A bug in TypeScript
Milestone

Comments

@mjbvz
Copy link
Contributor

mjbvz commented Aug 7, 2018

From @Floriferous on August 6, 2018 20:25

When composing a class together through mixins, the intellisense autocompletion does not work properly:

// @flow
/* eslint-env mocha */
import { expect } from 'chai';
import { compose } from 'recompose';

class RootCalculator {
  constructor() {
    this.initializer = 2;
  }

  add(a: number, b: number): number {
    return a + b;
  }

  returnVarParent() {
    return this.var;
  }

  return2(): number {
    return 2;
  }

  parentFunc() {
    return this.childFunc();
  }
}

const withCalc2 = (SuperClass = class {}) =>
  class extends SuperClass {
    constructor() {
      super();
      this.initializer = this.initializer * 2;
    }

    increment(a: number): number {
      return a + 1;
    }

    setVar(value): void {
      this.var = value;
    }
  };

const withCalc3 = SuperClass =>
  class extends SuperClass {
    constructor() {
      super();
      this.initializer = this.initializer + 1;
    }

    multiply(a: number, b: number): number {
      return a * b;
    }

    returnVarChild(): void {
      return this.var;
    }

    return2(): number {
      return -2;
    }

    childFunc() {
      return 'it works';
    }
  };

const UberClass = compose(
  withCalc3,
  withCalc2,
)(RootCalculator);

const calculator = new UberClass();

calculator. // Wait for autocomplete, it can't find the class methods, or suggests them after plenty of other stuff

screen shot 2018-08-06 at 22 24 33

Copied from original issue: microsoft/vscode#55910

@mjbvz mjbvz self-assigned this Aug 7, 2018
@mjbvz mjbvz added the Domain: JavaScript The issue relates to JavaScript specifically label Aug 7, 2018
@mjbvz mjbvz removed their assignment Aug 7, 2018
@mjbvz mjbvz removed the Domain: JavaScript The issue relates to JavaScript specifically label Aug 7, 2018
@mjbvz
Copy link
Contributor Author

mjbvz commented Aug 7, 2018

Almost certainly out of scope. We'd essentially have to evaluate the program to provide proper intellisense here

@ghost
Copy link

ghost commented Aug 7, 2018

Actually, this should be doable with the right type annotations!

class RootCalculator {
    constructor() {
        this.initializer = 2;
    }

    rootMethod() {}
}


/**
 * @template {new (...args: any[]) => { initializer: number }} T
 * @param {T} SuperClass
 */
function myMixin(SuperClass) {
    return class extends SuperClass {
        constructor(...args) {
            super(...args);
            this.initializer = this.initializer + 1;
        }

        mixinMethod() {}
    }
};

const UberClass = myMixin(RootCalculator);

const calculator = new UberClass();
calculator.initializer;
calculator.rootMethod(); // no error
calculator.someMethod(); // no error

Unfortunately this currently crashes...

@ghost ghost added the Bug A bug in TypeScript label Aug 7, 2018
@ghost
Copy link

ghost commented Aug 7, 2018

This is just a fountain of bugs: #26283, #26284, #26286. Then it should work.

@ghost ghost added the Fixed A PR has been merged for this issue label Aug 8, 2018
@Floriferous
Copy link

Floriferous commented Aug 8, 2018

Amazing! 💯

Here are a few tests I used to make sure I understand these mixins well, there might be other stuff hidden in it:

Test File
// @flow
/* eslint-env mocha */
import { expect } from 'chai';
import { compose } from 'recompose';

class RootCalculator {
  constructor() {
    this.initializer = 2;
  }

  add(a: number, b: number): number {
    return a + b;
  }

  returnVarParent() {
    return this.var;
  }

  return2(): number {
    return 2;
  }

  parentFunc() {
    return this.childFunc();
  }
}

const addClass = SuperClass => class extends SuperClass {};

const withCalc2 = (SuperClass = class {}) =>
  class extends SuperClass {
    constructor() {
      super();
      this.initializer = this.initializer * 2;
    }

    increment(a: number): number {
      return a + 1;
    }

    setVar(value): void {
      this.var = value;
    }
  };

const withCalc3 = SuperClass =>
  class extends SuperClass {
    constructor() {
      super();
      this.initializer = this.initializer + 1;
    }

    multiply(a: number, b: number): number {
      return a * b;
    }

    returnVarChild(): void {
      return this.var;
    }

    return2(): number {
      return -2;
    }

    childFunc() {
      return 'it works';
    }
  };

const UberClass = compose(
  withCalc3,
  withCalc2,
)(RootCalculator);

describe('Class composition', () => {
  it('extends properly', () => {
    class MyClass extends RootCalculator {}

    expect(new MyClass().add(1, 2)).to.equal(3);
  });

  it('combines classes with the mixin', () => {
    const Calc2 = withCalc2();
    class MyClass extends addClass(Calc2) {}

    expect(new MyClass().increment(1)).to.equal(2);
  });

  it('combines multiple classes', () => {
    class MyClass extends withCalc3(withCalc2(RootCalculator)) {}

    expect(new MyClass().increment(1)).to.equal(2);
    expect(new MyClass().add(1, 2)).to.equal(3);
    expect(new MyClass().multiply(2, 3)).to.equal(6);
  });

  it('combines multiple classes with compose', () => {
    class MyClass extends compose(
      withCalc3,
      withCalc2,
    )(RootCalculator) {}

    expect(new MyClass().increment(1)).to.equal(2);
    expect(new MyClass().add(1, 2)).to.equal(3);
    expect(new MyClass().multiply(2, 3)).to.equal(6);
  });

  it('combines multiple classes with compose hidden away', () => {
    const MyClass = class extends UberClass {};

    expect(new MyClass().increment(1)).to.equal(2);
    expect(new MyClass().add(1, 2)).to.equal(3);
    expect(new MyClass().multiply(2, 3)).to.equal(6);
  });

  it('calls the constructor chain from Root to Calc3', () => {
    class MyClass extends UberClass {}

    expect(new MyClass().initializer).to.equal(5);
  });

  it('autocompletion works', () => {
    class MyClass extends compose(
      withCalc3,
      withCalc2,
    )(RootCalculator) {}

    const Calc = new MyClass();

    // Play around here..
  });

  it('can use instance variables across calculators', () => {
    class MyClass extends UberClass {}

    const Calc = new MyClass();

    expect(Calc.returnVarParent()).to.equal(undefined);
    expect(Calc.returnVarChild()).to.equal(undefined);

    Calc.setVar(2);

    expect(Calc.var).to.equal(2);
    expect(Calc.returnVarParent()).to.equal(2);
    expect(Calc.returnVarChild()).to.equal(2);
  });

  it('overwrites parent methods ', () => {
    class MyClass extends UberClass {
      add(a, b) {
        return a - b;
      }
    }

    expect(new MyClass().add(2, 3)).to.equal(-1);
  });

  it('overwrites in the right order', () => {
    expect(new UberClass().return2()).to.equal(-2);
  });

  it('can call super', () => {
    class MyClass extends UberClass {
      add = super.multiply;

      increment(a) {
        return super.increment(a - 1);
      }
    }

    expect(new MyClass().add(2, 3)).to.equal(6);
    expect(new MyClass().increment(2)).to.equal(2);
  });

  it('shows the right types with flow-type', () => {
    class MyClass extends UberClass {
      return2(): string {
        return '2';
      }

      returnNumber() {
        return super.return2();
      }
    }

    // Inspect types here to make sure they are correct
    expect(new MyClass().return2()).to.equal('2');
    expect(new MyClass().returnNumber()).to.equal(-2);
  });

  it('parent can call child funcs', () => {
    expect(new UberClass().parentFunc()).to.equal('it works');
  });
});

@RyanCavanaugh
Copy link
Member

@sandersn looks like this has been split out into some sub-bugs but please track completion of the entire scenario

@sandersn sandersn removed the Fixed A PR has been merged for this issue label Aug 13, 2018
@sandersn sandersn modified the milestones: TypeScript 3.4.0, Backlog Mar 13, 2019
@sandersn sandersn removed their assignment Jan 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants