Skip to content
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

The 'thisArg' parameter should be used to type 'this' in callbacks to map, filter, etc. #12548

Closed
e-cloud opened this issue Nov 29, 2016 · 4 comments
Labels
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Fixed A PR has been merged for this issue Help Wanted You can do this

Comments

@e-cloud
Copy link
Contributor

e-cloud commented Nov 29, 2016

TypeScript Version: (2.2.0-dev.20161128)

Code

tets.ts

class A {
    options: any[]
    addOptions(options: any[]) {
        if (!this.options) {
            this.options = [];
        }
        options.forEach(function (item) {
            this.options.push(item);
        }, this);
        return this;
    }
}
tsc --module commonjs --noImplicitThis --noEmit test.ts

Expected behavior:
compile with no error output

Actual behavior:
test.ts(8,13): error TS2683: 'this' implicitly has type 'any' because it does not have a type annotation.

As some built-in method of array can assign this, could it be inferred through the scopes?

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Nov 29, 2016

forEach should actually contextually type this based on the thisArg, and that will work.

But thisArg isn't used for the callback types in lib.d.ts's declarations for forEach, map, filter, etc. right now.

@DanielRosenwasser DanielRosenwasser added Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript labels Nov 29, 2016
@DanielRosenwasser DanielRosenwasser changed the title --noImplicitThis option incompatible with some built-in types' method The 'thisArg' parameter should be used to type 'this' in callbacks to map, filter, etc. Nov 29, 2016
@mhegazy mhegazy added this to the Community milestone Dec 7, 2016
@mhegazy mhegazy added the Help Wanted You can do this label Dec 7, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Dec 7, 2016

PRs welcomed. You can find more information about contributing lib.d.ts fixes at https://github.com/Microsoft/TypeScript/blob/master/CONTRIBUTING.md#contributing-libdts-fixes.

@e-cloud
Copy link
Contributor Author

e-cloud commented Dec 7, 2016

@mhegazy I've fixed the source. But how can i write the tests for changes? The test suite is so huge, it could take time to figure out. So ask you for how is more direct.

e-cloud added a commit to e-cloud/TypeScript that referenced this issue Dec 9, 2016
when enabling noImplicitThis, if assing this argument for
methods like `array.forEach` will cause compile error.
This commit fixes it.

fix microsoft#12548
e-cloud added a commit to e-cloud/TypeScript that referenced this issue Dec 17, 2016
when enabling noImplicitThis, if assing this argument for
methods like `array.forEach` will cause compile error.
This commit fixes it.

fix microsoft#12548

Note: just use some regexp to batchly update the interfaces:
1. search: `(some|every|forEach|filter|map|find|findindex)(\(.+?:
\()(value: .+?, index: number, .+?: .+?\) => .+?, thisArg\?: )any(\):
.+?;)`
replacement: `$1<Z>$2this: Z, $3Z$4`
2. search: `(some|every|forEach|filter|map|find|findindex)<(.+?)>(\(.+?:
\()(value: .+?, index: number, .+?: .+?\) => .+?, thisArg\?: )any(\):
.+?;)`
replacement: `$1<Z, $2>$3this: Z, $4Z$5`
e-cloud added a commit to e-cloud/TypeScript that referenced this issue Jan 5, 2017
when enabling `noImplicitThis`, if assing this argument for
methods like `array.forEach` will cause compilation error.
This commit fixes it.

fix microsoft#12548
e-cloud added a commit to e-cloud/TypeScript that referenced this issue Jan 11, 2017
when enabling `noImplicitThis`, if assing this argument for
methods like `array.forEach` will cause compilation error.
This commit fixes it.

fix microsoft#12548
e-cloud added a commit to e-cloud/TypeScript that referenced this issue Jan 22, 2017
when enabling `noImplicitThis`, if assing this argument for
methods like `array.forEach` will cause compilation error.
This commit fixes it.

fix microsoft#12548
e-cloud added a commit to e-cloud/TypeScript that referenced this issue Jan 24, 2017
when enabling `noImplicitThis`, if assing this argument for
methods like `array.forEach` will cause compilation error.
This commit fixes it.

fix microsoft#12548
e-cloud added a commit to e-cloud/TypeScript that referenced this issue Feb 8, 2017
when enabling `noImplicitThis`, if assing this argument for
methods like `array.forEach` will cause compilation error.
This commit fixes it.

fix microsoft#12548
e-cloud added a commit to e-cloud/TypeScript that referenced this issue Mar 9, 2017
when enabling `noImplicitThis`, if assing this argument for
methods like `array.forEach` will cause compilation error.
This commit fixes it.

fix microsoft#12548
@mhegazy mhegazy modified the milestones: TypeScript 2.3, Community Mar 10, 2017
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Mar 10, 2017
@mhegazy mhegazy mentioned this issue Jun 4, 2017
@jcalz
Copy link
Contributor

jcalz commented Jul 25, 2017

Should this be reopened in light of #16223?

@microsoft microsoft locked and limited conversation to collaborators Jul 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Fixed A PR has been merged for this issue Help Wanted You can do this
Projects
None yet
Development

No branches or pull requests

4 participants