Skip to content

Restrict base-class-first error to class declaration bases#11349

Merged
RyanCavanaugh merged 1 commit into
microsoft:masterfrom
RyanCavanaugh:fix10624
Oct 19, 2016
Merged

Restrict base-class-first error to class declaration bases#11349
RyanCavanaugh merged 1 commit into
microsoft:masterfrom
RyanCavanaugh:fix10624

Conversation

@RyanCavanaugh
Copy link
Copy Markdown
Member

Fixes all known repros of #10624

Comment thread src/compiler/checker.ts
if (baseType.symbol.valueDeclaration && !isInAmbientContext(baseType.symbol.valueDeclaration)) {
if (baseType.symbol.valueDeclaration &&
!isInAmbientContext(baseType.symbol.valueDeclaration) &&
baseType.symbol.valueDeclaration.kind === SyntaxKind.ClassDeclaration) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what about:

class C extends makeClass() {}
var makeClass = function() {...}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We don't know whether the var was meaningfully hoisted out or not

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

functions are hoisted, vars are not. so we could add another clause for functions as well. that is all what i meant.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Their declarations are hoisted, which means they might be initialized earlier

makeClass = function() { ... }
class C extends makeClass() {}
var makeClass = function() {...}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We could check for let / const though

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

const/let should give you an error already if used before declaration.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

so look like it is not worth the change then.. nevermind.

@RyanCavanaugh RyanCavanaugh merged commit c4ef5f3 into microsoft:master Oct 19, 2016
@RyanCavanaugh RyanCavanaugh deleted the fix10624 branch October 19, 2016 18:47
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants