GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Allow expressions in class extends clauses by ahejlsberg · Pull Request #3516 · microsoft/TypeScript

Conversation

Anders Hejlsberg (ahejlsberg) commented Jun 15, 2015

Copy link

Copy Markdown

Member

This PR allows the extends clause of a class to specify an arbitrary expression that computes a constructor function. The PR also relaxes the strict requirement that extends specify a class type and instead allows expressions of "class-like" constructor function types. This means that built-in types can now be extended in class declarations.

The extends clause of a class previously required a type reference to be specified. It now accepts an expression optionally followed by a type argument list. The type of the expression must be a constructor function type with at least one construct signature that has the same number of type parameters as the number of type arguments specified in the extends clause. The return type of the matching construct signature(s) is the base type from which the class instance type inherits. Effectively, this allows both real classes and "class-like" expressions to be specified in the extends clause.

Some examples:

// Extend built-in types

class MyArray extends Array<number> { }
class MyError extends Error { }

// Extend computed base class

class ThingA {
    getGreeting() { return "Hello from A"; }
}

class ThingB {
    getGreeting() { return "Hello from B"; }
}

interface Greeter {
    getGreeting(): string;
}

interface GreeterConstructor {
    new (): Greeter;
}

function getGreeterBase(): GreeterConstructor {
    return Math.random() >= 0.5 ? ThingA : ThingB;
}

class Test extends getGreeterBase() {
    sayHello() {
        console.log(this.getGreeting());
    }
}

Fixes #511.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Anders Hejlsberg (ahejlsberg) added 7 commits

June 14, 2015 07:10


          Allow expressions in extends clause of class

dfa1494


          Use instantiated signatures in super calls

80ea687


          Accepting new baselines

956d73e


          Check that base constructor return types are identical

cc81cc7

Treat class extends clause as expression position in services.ts


          Accepting new baselines

367e257


          Adding basic tests

d575259


          Accepting new baselines

186f525

Microsoft Contribution License Agreements (msftclas) added the cla-not-required label

Jun 15, 2015

CyrusNajmabadi reviewed

Jun 16, 2015

View reviewed changes

Comment thread

tests/baselines/reference/aliasUsageInTypeArgumentOfExtendsClause.types Outdated

Cyrus Najmabadi (CyrusNajmabadi) Jun 16, 2015

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The best way to handle all these changes is to tweak typeWriter to produce results like what we used to get. Then, we can actually review the handful of files that have relevant changes. Then, once we can see that nothing else changed unintentionally, we can remove that tweak in a followup change and then update all the baselines. With several hundred files changed, github won't show all the changes. And it becomes very difficult to ascertain if something else might have broke as part of this.

We've done this numerous times as we've changed hte compiler. For example, even when moving over from the old compiler to the new one, we tweaked things in the new typewriter to produce the same output as hte previous one.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Anders Hejlsberg (ahejlsberg) Jun 16, 2015

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a workaround to reduce the number of changes, but it isn't possible to completely eliminate the differences. Should be a lot better now, though.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Anders Hejlsberg (ahejlsberg) added 6 commits

June 16, 2015 06:54


          Workaround to match previous type writer output

e305de1


          Accepting new baselines

d09d61f


          Adding comments and doing a bit of renaming

de8eb22


          Adding another comment

38e3d9f


          Merge branch 'master' into extendsExpressions

2c57776

Conflicts:
	src/compiler/checker.ts
	tests/baselines/reference/strictModeReservedWordInClassDeclaration.errors.txt


          Accepting new baselines after merge

f6bcf70

Anders Hejlsberg (ahejlsberg) commented Jun 16, 2015

Copy link

Copy Markdown

Member Author

Mohamed Hegazy (@mhegazy) Jason Freeman (@JsonFreeman) Cyrus Najmabadi (@CyrusNajmabadi) I'd like to get this one in pretty soon. I will be putting up a separate CR with the change to __extends to permit the base class to be null since it generates a lot of differences.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Anders Hejlsberg (ahejlsberg) mentioned this pull request

Jun 16, 2015

Support null in __extends helper #3524

Merged

aozgaa reviewed

Jun 16, 2015

View reviewed changes

Comment thread

src/compiler/checker.ts

Arthur Ozga (aozgaa) Jun 16, 2015

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can/should this be a JSDoc-style comment?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Jason Freeman (JsonFreeman) Jun 16, 2015

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Jason Freeman (JsonFreeman) commented Jun 16, 2015

Copy link

Copy Markdown

Contributor

I can take a look later today, or tonight.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JsonFreeman reviewed

Jun 17, 2015

View reviewed changes

Comment thread

src/compiler/types.ts

Jason Freeman (JsonFreeman) Jun 17, 2015

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't these still need to be deferred until after getDeclaredTypeOfSymbol?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Jason Freeman (JsonFreeman) Jun 17, 2015

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In other words, why remove InterfaceWithBaseTypes?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Anders Hejlsberg (ahejlsberg) Jun 17, 2015

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, they're still deferred. The pattern we use elsewhere for single deferred values is a resolvedXXX property guarded by a getXXX function that returns the value. In cases where we defer the computation of multiple properties we use an XXXWithYYY type and return the object itself.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

JsonFreeman reviewed

Jun 17, 2015

View reviewed changes

Comment thread

src/compiler/checker.ts Outdated

Jason Freeman (JsonFreeman) Jun 17, 2015

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was just computed above on line 10591. Is it possible to reuse it?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Anders Hejlsberg (ahejlsberg) Jun 17, 2015

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, here we're looking at the instantiated constructors. We could potentially cache the uninstantiated constructors, but I don't think it is worth the effort.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Jason Freeman (JsonFreeman) Jun 17, 2015

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh true, I missed that.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Anders Hejlsberg (ahejlsberg) added 2 commits

June 17, 2015 13:28


          Addressing CR feedback

d71af8a


          Accepting new baselines

471f6e0

Jason Freeman (JsonFreeman) commented Jun 17, 2015

Copy link

Copy Markdown

Contributor

👍

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Anders Hejlsberg (ahejlsberg) added 3 commits

June 17, 2015 14:04


          Addressing more CR feedback

efcccaa


          Merge branch 'master' into extendsExpressions

26fd879


          Accepting new baselines after master merge

5b9a1b5

Anders Hejlsberg (ahejlsberg) added a commit that referenced this pull request

Jun 17, 2015


          Merge pull request #3516 from Microsoft/extendsExpressions

247d582

Allow expressions in class extends clauses

Anders Hejlsberg (ahejlsberg) merged commit 247d582 into master

Jun 17, 2015

Anders Hejlsberg (ahejlsberg) deleted the extendsExpressions branch

June 17, 2015 21:27

Anders Hejlsberg (ahejlsberg) mentioned this pull request

Jun 17, 2015

Can not extend built in types #1168

Closed

Basarat Ali Syed (basarat) mentioned this pull request

Jun 23, 2015

Convert HTMLElement and friends to classes to support Web Components/Custom elements #574

Open

Sophia J. Turner (sophiajt) mentioned this pull request

Aug 25, 2015

'extends null' doesn't error on using methods from Object.prototype #4454

Closed

Dirk Bäumer (dbaeumer) mentioned this pull request

Oct 2, 2015

Extending from Error doesn't work correctly under tsc 1.6.2 #5069

Closed

Joe Duncko (JoeDuncko) mentioned this pull request

Oct 13, 2015

Make variable declarations open ended #819

Closed

Ian Bytchek (iby) mentioned this pull request

Mar 22, 2016

Cannot extend native Error class #7639

Closed

malibuzios mentioned this pull request

Apr 4, 2016

Proposal: error on invalid assignments between classes that share a nominal heritage #7817

Closed

Andy1605 commented Apr 11, 2016

Copy link

Copy Markdown

Hey there!

Is this still a thing? I just tried to extend a Map getting the "calling constructor without new is forbidden" error! Here is a fiddle.

Or did i miss something in the many discussions?

Thanks in advance.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Ryan Cavanaugh (RyanCavanaugh) commented Apr 11, 2016

Copy link

Copy Markdown

Member

Andy1605 this is a bug in your runtime; see https://bugs.chromium.org/p/v8/issues/detail?id=3101 and https://bugs.chromium.org/p/v8/issues/detail?id=3622 and google/traceur-compiler#1413 (comment)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Andy1605 commented Apr 12, 2016

Copy link

Copy Markdown

Tested it with chrome 50.0.2661.66 beta-m (64-bit), Cyberfox 45.0.2 (64-bit) and with Opera 36.0.2130.46 (32 bit - Chrome engine, bad example), Edge 25.10586.0.0. Same result.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Andy1605 commented Apr 13, 2016

Copy link

Copy Markdown

Is there a plausible workaround for this problem on TypeScript end? This is game changing for us!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mohamed Hegazy (mhegazy) commented Apr 13, 2016

Copy link

Copy Markdown

Contributor

Andy1605 what is the proposed output that would make something like this work without engine support?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Andy1605 commented Apr 14, 2016

Copy link

Copy Markdown

Mohamed Hegazy (@mhegazy) Hmm... would it make sense, if there would be a possibility to suppress the "super() call must be there" error? Then one could at least try to write a workaround. Smth like creating an instance of the subclassed type and merging it back. I am lost here as well.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

René Verheij (flyon) commented Sep 11, 2016 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Ryan Cavanaugh (@RyanCavanaugh) that chromium issue seems fixed now (https://bugs.chromium.org/p/v8/issues/detail?id=3101)

Both in Chrome and Chrome canary I can do the following in the chrome console without errors:

class ResourceMap extends Map {}
var x = new ResourceMap();
x.set('foo',true);

However when I execute the compiled typescript output I get a similar error as Andy1605: Uncaught TypeError: Constructor Map requires 'new'

var __extends = (this && this.__extends) || function (d, b) {
    for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];
    function __() { this.constructor = d; }
    d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
};
var ResourceMap = (function (_super) {
    __extends(ResourceMap, _super);
    function ResourceMap() {
        _super.apply(this, arguments);
    }
    return ResourceMap;
}(Map));

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

René Verheij (flyon) mentioned this pull request

Sep 11, 2016

Extending ES6 Map causes error - Constructor Map requires 'new' #10853

Closed

Microsoft (microsoft) locked and limited conversation to collaborators

Jun 19, 2018

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

9 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

Anders Hejlsberg (ahejlsberg) commented Jun 15, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Anders Hejlsberg (ahejlsberg) commented Jun 16, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jason Freeman (JsonFreeman) commented Jun 16, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jason Freeman (JsonFreeman) commented Jun 17, 2015

Uh oh!

Andy1605 commented Apr 11, 2016

Uh oh!

Ryan Cavanaugh (RyanCavanaugh) commented Apr 11, 2016

Uh oh!

Andy1605 commented Apr 12, 2016

Uh oh!

Andy1605 commented Apr 13, 2016

Uh oh!

Mohamed Hegazy (mhegazy) commented Apr 13, 2016

Uh oh!

Andy1605 commented Apr 14, 2016

Uh oh!

René Verheij (flyon) commented Sep 11, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

René Verheij (flyon) commented Sep 11, 2016 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.