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

TypeScript 2.9 incorrect generator this value in ES5 #24564

Closed
zenorbi opened this issue Jun 1, 2018 · 4 comments
Closed

TypeScript 2.9 incorrect generator this value in ES5 #24564

zenorbi opened this issue Jun 1, 2018 · 4 comments
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@zenorbi
Copy link

zenorbi commented Jun 1, 2018

TypeScript Version: Version 3.0.0-dev.20180601 and also Version 2.9.1

Search Terms: ES5 generator this value

Code

class Test {
	async getChildren() {
		const randomItems = await this.someRandomMethod();

		const processedItems = randomItems.map((item) => {
			return this.doSomeProcessing(item); // <-- this will have incorrect _this value!
		});

		return ([] as string[]).concat(...processedItems); // <-- if I don't cast here, the above _this is good
	}

	private doSomeProcessing(stuff: string) {
		return stuff;
	}

	private async someRandomMethod() {
		return ["a", "b", "c"];
	}
}

Expected behavior:
this.doSomeProcessing(item) points to a correct this value

Actual behavior:
The emitted code has var _this = this but in the wrong scope

Playground Link:
http://www.typescriptlang.org/play/index.html#src=class%20Test%20%7B%0D%0A%09async%20getChildren()%20%7B%0D%0A%09%09const%20randomItems%20%3D%20await%20this.someRandomMethod()%3B%0D%0A%0D%0A%09%09const%20processedItems%20%3D%20randomItems.map((item)%20%3D%3E%20%7B%0D%0A%09%09%09return%20this.doSomeProcessing(item)%3B%20%2F%2F%20%3C--%20this%20will%20have%20incorrect%20_this%20value!%0D%0A%09%09%7D)%3B%0D%0A%0D%0A%20%20%20%20%20%20%20%20return%20(%5B%5D%20as%20string%5B%5D).concat(...processedItems)%3B%20%2F%2F%20%3C--%20if%20I%20don't%20cast%20here%2C%20the%20above%20_this%20is%20good%0D%0A%20%20%20%20%20%20%20%20%2F%2Freturn%20(%5B%5D).concat(...processedItems)%3B%20%2F%2F%20%3C--%20no%20casting%20version%0D%0A%20%20%20%20%7D%0D%0A%0D%0A%20%20%20%20private%20doSomeProcessing(stuff%3A%20string)%20%7B%0D%0A%09%09return%20stuff%3B%0D%0A%09%7D%0D%0A%0D%0A%09private%20async%20someRandomMethod()%20%7B%0D%0A%09%09return%20%5B%22a%22%2C%20%22b%22%2C%20%22c%22%5D%3B%0D%0A%09%7D%0D%0A%7D%0D%0A%0D%0Aasync%20function%20testCalling()%20%7B%0D%0A%20%20%20%20try%20%7B%0D%0A%20%20%20%20%20%20%20%20const%20tester%20%3D%20new%20Test()%3B%0D%0A%20%20%20%20%20%20%20%20const%20children%20%3D%20await%20tester.getChildren()%3B%0D%0A%20%20%20%20%20%20%20%20alert(children)%3B%20%2F%2F%20Expecting%3A%20a%2C%20b%2C%20c%0D%0A%20%20%20%20%7D%20catch%20(err)%20%7B%0D%0A%20%20%20%20%20%20%20%20alert(err.stack)%3B%0D%0A%20%20%20%20%7D%0D%0A%7D%0D%0AtestCalling()%3B%0D%0A

@mhegazy mhegazy added the Bug A bug in TypeScript label Jun 1, 2018
@mhegazy mhegazy added this to the TypeScript 2.9.2 milestone Jun 1, 2018
@IanYates
Copy link

IanYates commented Jun 4, 2018

I got hit by this very issue

TypeScript 2.9.1

I'm trying to recreate it in Playground with some cut down but without luck so far.

However the issue is very similar - we've got an async function where this. is referenced in some code that ends up being in case 3 of the generated switch, however it's case 0 only receiving the required var _this = this; line in it (rather than it being outside of the async machinery)

@IanYates
Copy link

IanYates commented Jun 4, 2018

In case it helps, here's a diff of my Javascript code where the only difference was a change from TypeScript 2.8.3 to 2.9.1
That moved line makes a huge difference at runtime :)

image

@rbuckton
Copy link
Member

rbuckton commented Jun 4, 2018

This looks like it is due to a recent change to temporary variable emit. Between transformation steps the temp variables end up before the this capture, so the generator transform doesn't look far enough to find the statement to move it outside of the block.

@IanYates
Copy link

IanYates commented Jun 4, 2018

Thanks for investigating.

I audited our codebase and found one other situation where this also occurred (moving to inside a single case block rather than outside the generator) but ended up being harmless. The var _this = this line was moved in a few hundred other areas, but all still safe (it effectively swapped with the other var line of declared variables created by the TS compiler)

Happy that I've caught the sole break, but it was a tricky one to track down. Happy to provide more info if it helps you solve it, although I suspect the Playground link provided by @zenorbi is much more succinct than anything I can provide.

Looking forward to whatever you find :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

4 participants