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 preserves 'override' modifier in JavaScript output #43535

Closed
DanielRosenwasser opened this issue Apr 5, 2021 · 2 comments · Fixed by #43536
Closed

TypeScript preserves 'override' modifier in JavaScript output #43535

DanielRosenwasser opened this issue Apr 5, 2021 · 2 comments · Fixed by #43536
Assignees
Labels
Bug A bug in TypeScript Domain: Transforms Relates to the public transform API Fix Available A PR has been opened for this issue Good First Issue Well scoped, documented and has the green light Help Wanted You can do this

Comments

@DanielRosenwasser
Copy link
Member

I'm not sure where to ask this, but it seems like tsc is compiling and leaving the override keyword in the compiled result. Is this something other folks are seeing? It seems like it needs the signature to match exactly, or it will show up in the output, see here:

https://www.typescriptlang.org/play?ts=4.3.0-beta#code/LAKAxgNghgzjAEBlADgUwheBvUp72QFcAjCASzHgHNUAXAYQAsoAnGmACgEoAueAO0IBbYqhbY8+eCzqEW-eAAYA3JIC+uEPiKkK1OgBE5UWmQD2-bn0EixErVJm05Cles3aS5SmFi0r8ABuZmQAJtgaIJGgkLAIABJmEACeAILG8KgAHrSo-KEIKOiYOCCSZoFiLGGo+gzMbKicXPZS0rLy8ACMbg7RDhVVNXVGLCbmlrwCwqLipW1OLvAAzL34-fiDLNWhtb4w-lPBYa3w-WpAA

Originally posted by @seiyria in #2000 (comment)

@DanielRosenwasser DanielRosenwasser added Bug A bug in TypeScript Domain: Transforms Relates to the public transform API Good First Issue Well scoped, documented and has the green light Help Wanted You can do this labels Apr 5, 2021
@DanielRosenwasser
Copy link
Member Author

Basically if you're targeting anything ES2015 and newer, you'll get the override modifier.

@TimvdLippe
Copy link
Contributor

We hit this issue in Chrome DevTools while attempting to upgrade to 4.3.0 (tracked at https://bugs.chromium.org/p/chromium/issues/detail?id=1196203). DevTools uses a lot of @override annotations back from our Closure Compiler time. We discovered that some of the override annotations were leftover in .ts files. When fixing these issues, we ran into build process issues when rollup couldn't consume the compiled JavaScript files.

My assumption was that https://github.com/microsoft/TypeScript/blob/master/tests/cases/docker/chrome-devtools-frontend-next/Dockerfile would have caught this issue, but I think we might not have caught this as we are not checking whether the .js files actually run in the Dockerfile. I don't think it is a good idea to also check whether the compiled output runs, since that can introduce flakiness and I think the user tests are only concerning compilations, but that might be an option to look into.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: Transforms Relates to the public transform API Fix Available A PR has been opened for this issue Good First Issue Well scoped, documented and has the green light Help Wanted You can do this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants