GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — Typed 'this' in object literal methods by ahejlsberg · Pull Request #14141 · microsoft/TypeScript Conversation
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Latest commit adds contextual |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
The one issue that remains for discussion here is whether the strongly typed |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I like this change.
ThisType (Object.defineProperty, knockout and react), and a couple of places that show that this needs to be the intersection of the object literal type and the parameter type.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| } | ||
|
|
||
| function getThisTypeFromContextualType(type: Type): Type { | ||
| return applyToContextualType(type, t => { |
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
shouldn't applyToContextualType just be named mapType and replace mapType? It doesn't do anything specific to contextual types that I could see, and it's better than the current mapType because it skips return values of undefined. #Resolved
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Yes, that makes sense.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| tests/cases/conformance/fixSignatureCaching.ts(640,13): error TS2304: Cannot find name 'window'. | ||
| tests/cases/conformance/fixSignatureCaching.ts(641,13): error TS2304: Cannot find name 'window'. | ||
| tests/cases/conformance/fixSignatureCaching.ts(704,18): error TS2339: Property 'prepareDetectionCache' does not exist on type '{}'. | ||
| tests/cases/conformance/fixSignatureCaching.ts(704,45): error TS2339: Property '_cache' does not exist on type '{ constructor: (userAgent: any, maxPhoneWidth: any) => void; mobile: () => any; phone: () => any; tablet: () => any; userAgent: () => any; userAgents: () => any; os: () => any; version: (key: any) => any; versionStr: (key: any) => any; is: (key: any) => any; match: (pattern: any) => any; isPhoneSized: (maxPhoneWidth: any) => any; mobileGrade: () => any; }'. |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
The purpose of this test isn't obvious without following the pointer back to issue #10697. Can you add one more comment at the top of the file like "this test originally failed by using 100% CPU and should now take less than 1 second to run". I'd like a faster way to know that these additional failures are not a problem.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| this // this: containing object literal type | ||
| this.mine | ||
| this.willDestroy | ||
| //this.willDestroy |
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
just delete this, I think. #Resolved
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Done.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Summary of RWC breaks [1]: two good breaks from contextual typing of function assignment. Two breaks that show that [1] Note that I haven't looked at all of RWC yet; it hit a stack overflow halfway through.
declare function f(elt: HTMLElement): void; function outer() { var self = this; var args: Draggable; args.helper = function () { return f(this); // error 'this: Draggable' is not assignable to 'HTMLElement' // Property 'accessKey' is missing in 'Draggable' }; } This catches code that is just wrong. I looked at (This happened in two different projects.)
// Override the prototype method on C CBase.prototype.debug = function() { var self: C = this; // error type 'CBase' is not assignable to 'C' // Property '_subclassProperty' is missing in 'CBase' I'm not sure what to make of this. The comment makes it sound like the intention is to override the method on C, not on CBase. So probably this is a good error too.
Object.defineProperties(Array.prototype, { empty: { value: function() { return this.length === 0; } } }) It looks like
function defineProperty(c: any, name: string, defaultValue: any) { var backingFieldName = "_" + name; Object.defineProperty( c.prototype, name, { get: function(): any { if (typeof this[backingFieldName] === 'undefined') { this[backingFieldName] = defaultValue; } return this[backingFieldName]; }, // code continues ... }); }; This would also be fixed by adding
interface MockJaxOptions { url?: string; responseTime?: number; status?: number; responseText?: string; responseHeaders: StringMap<string>; } $.mockjax({ type: "GET", url: "blah/blah/blah", status: 200, contentType: "application/json", response: function() { this.responseText = JSON.stringify({ stuff: "things" }); // error: property 'responseText' does not exist on '{ type: string ... }' } }); This is bad because |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
More breaks:
public getEmitOutput(resource: URI, absoluteWorkersResourcePath: string): WinJS.TPromise<Modes.IEmitOutput> { // TODO@Ben technical debt: worker cannot resolve paths absolute let model = this.resourceService.get(resource); let cssLinks: string[] = this.cssLinks || []; // Custom Renderer to fix href in images let renderer = new Marked.marked.Renderer(); let $this = this; renderer.image = function(href: string, title: string, text: string): string { let out = '<img src="' + $this.fixHref(resource, href) + '" alt="' + text + '"'; if (title) { out += ' title="' + title + '"'; } out += (this.options && this.options.xhtml) ? '/>' : '>'; ~~~~~~~ !!! error TS2339: Property 'options' does not exist on type 'MarkedRenderer'. ~~~~~~~ !!! error TS2339: Property 'options' does not exist on type 'MarkedRenderer'. This seems like a good break. I can't find a property named 'options' in
ko.observable.fn['toInt'] = function() { return parseInt(this()); ~~~~~~ !!! error TS2349: Cannot invoke an expression whose type lacks a call signature. Type 'KnockoutObservableFunctions<any>' has no compatible call signatures. } This seems like an easy fix to knockout's d.ts. It's just that nobody ever noticed that
private setupEvents(xhr: XMLHttpRequest, request: AjaxRequest) { const progressSubscriber = request.progressSubscriber; xhr.ontimeout = function xhrTimeout(e) { const {subscriber, progressSubscriber, request } = (<any>xhrTimeout); if (progressSubscriber) { progressSubscriber.error(e); } subscriber.error(new AjaxTimeoutError(this, request)); //TODO: Make betterer. ~~~~ !!! error TS2345: Argument of type 'XMLHttpRequestEventTarget' is not assignable to parameter of type 'XMLHttpRequest'. !!! error TS2345: Property 'onreadystatechange' is missing in type 'XMLHttpRequestEventTarget'. }; This seems like a good break too. The comment even indicates that the author is suspicious that something is wrong.
export function main() { describe('es5 decorators', () => { it('should declare directive class', () => { var MyDirective = Directive({}).Class({constructor: function() { this.works = true; }}); ~~~~~ !!! error TS2339: Property 'works' does not exist on type '{ constructor: () => void; }'. expect(new MyDirective().works).toEqual(true); }); This is probably another case where the type of the object literal needs to be intersected with the contextual type from the call signature. But it's in a test. It could just be a completely dynamic use of an undeclared property.
// tree benchmark in React import {getIntParameter, bindAction} from 'angular2/src/test_lib/benchmark_util'; import * as React from './react.min'; var TreeComponent = React.createClass({ displayName: 'TreeComponent', render: function() { var treeNode = this.props.treeNode; ~~~~~ !!! error TS2339: Property 'props' does not exist on type '{ displayName: string; render: () => any; }'. This is evidence that |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
I just tried this out and thought this would work: But it looks like the getter isn't aware of the type of |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Braden Snell (@bradenhs) Nope, that is not correct. But the code right now specifically works with the contextual type of |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
With latest commits we now properly type |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| @@ -22,9 +21,6 @@ tests/cases/conformance/types/thisType/thisTypeInAccessorsNegative.ts(16,22): er | |||
| } | |||
| const contextual: Foo = { | |||
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Can you move this test case to thisTypeInAccessors.ts so that we can see the types?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Latest commit puts all of the new functionality under the |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
In the else if (type.flags & TypeFlags.TypeParameter && (type as TypeParameter).isThisType) { if (inObjectTypeLiteral) { writer.reportInaccessibleThisError(); } writer.writeKeyword("this"); } |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Arthur Ozga (@aozgaa) No, that is an unrelated check (it reports an error if |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Mohamed Hegazy (@mhegazy) I think this one is good to do. Want to take a look? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
In this scenario, is it possible to infer |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I haven’t tested it but it should be possible with generics. declare function wrap<T, F extends (this: T) => any>(f: F): F |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Unfortunately that doesn't seem to work without specifying |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.