GV

GeezerVison

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

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — JavaScript prototype class inference by RyanCavanaugh · Pull Request #5876 · microsoft/TypeScript

Conversation

@RyanCavanaugh

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) commented Dec 1, 2015

Copy link
Copy Markdown
Member

Cleanup of #5578 that is based to the appropriate branch

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Ryan Cavanaugh (RyanCavanaugh) added 8 commits October 26, 2015 15:42
@RyanCavanaugh
JavaScript class inference from prototype property assignment
b31b45f
@RyanCavanaugh
JS class members as methods
bc3d95c
@RyanCavanaugh
Inference from JavaScript prototype property assignments
3b72131
@RyanCavanaugh
Human-readable fourslash debug output for completion lists / quickinfo
a3a5c16
@RyanCavanaugh
Fix TC
eaeeb1f
@RyanCavanaugh
WIP
fb83ee0
@RyanCavanaugh
Merge branch 'master' into javaScriptPrototypes
c3b59d1 
# Conflicts:
#	src/compiler/binder.ts
#	src/compiler/checker.ts
#	src/harness/fourslash.ts
@RyanCavanaugh
Merge fixup
c4b0b62
@msftclas Microsoft Contribution License Agreements (msftclas) added the cla-not-required label Dec 1, 2015
@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) mentioned this pull request Dec 1, 2015
Class inference from prototypes in JavaScript #5578
Closed
ahejlsberg
ahejlsberg reviewed Dec 4, 2015
View reviewed changes
Comment thread src/compiler/binder.ts Outdated

@ahejlsberg Anders Hejlsberg (ahejlsberg) Dec 4, 2015

Copy link
Copy Markdown
Member

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 doesn't guarantee that the prototype assignment immediately follows. Also, what if classId denotes a non-function, such as a variable or an interface declaration? I'm thinking you should instead check that the prototype assignment is immediately contained in a statement list and that the immediately preceding statement is a function declaration.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Ryan Cavanaugh (RyanCavanaugh) added 3 commits December 4, 2015 14:11
@RyanCavanaugh
JS Prototypes WIP
fabc43d
@RyanCavanaugh
Merge remote-tracking branch 'upstream/master' into javaScriptPrototypes
6bb62d6
@RyanCavanaugh
Simplified JS prototype class inference
fcd00a5
@ahejlsberg

Anders Hejlsberg (ahejlsberg) commented Dec 7, 2015

Copy link
Copy Markdown
Member

I'm thinking that the kind of inference you're doing should also apply when you access this in functions associated with the inferred "class". Specifically, within the constructor function and within methods (functions assigned to Foo.prototype.xxx), the type of this should be the inferred type. It seems odd to only infer the type outside the functions but not within them.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@RyanCavanaugh
Support 'this' in inferred method bodies
c97dfff
@RyanCavanaugh

Ryan Cavanaugh (RyanCavanaugh) commented Dec 7, 2015

Copy link
Copy Markdown
Member Author

Added this support for inferred method bodies, and added tests to verify that we don't require prototype assignments to do class inference
All reactions

Sorry, something went wrong.

Uh oh!

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

Ryan Cavanaugh (RyanCavanaugh) commented Dec 9, 2015

Copy link
Copy Markdown
Member Author

Anders Hejlsberg (@ahejlsberg) any other comments?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ahejlsberg
ahejlsberg reviewed Dec 9, 2015
View reviewed changes
Comment thread src/compiler/checker.ts Outdated

@ahejlsberg Anders Hejlsberg (ahejlsberg) Dec 9, 2015

Copy link
Copy Markdown
Member

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.

You need to create this type once and cache it. Right now you're creating a brand new type every time which allocates a lot of objects and defeats all the type comparison caching we do.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Ryan Cavanaugh (RyanCavanaugh) added 3 commits December 9, 2015 11:16
@RyanCavanaugh
Address CR feedback
50892ac
@RyanCavanaugh
One more
fcfc424
@RyanCavanaugh
Check for function flag on class symbol
37f3ff8
@ahejlsberg

Anders Hejlsberg (ahejlsberg) commented Dec 14, 2015

Copy link
Copy Markdown
Member

👍
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Ryan Cavanaugh (RyanCavanaugh) added a commit that referenced this pull request Dec 14, 2015
@RyanCavanaugh
Merge pull request #5876 from RyanCavanaugh/javaScriptPrototypes
2f447ee 
JavaScript prototype class inference
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

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.

4 participants

@RyanCavanaugh @ahejlsberg @DanielRosenwasser @msftclas
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.