-
Couldn't load subscription status.
- Fork 13.1k
abstract properties are now properly elided from imports #62673
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
base: main
Are you sure you want to change the base?
abstract properties are now properly elided from imports #62673
Conversation
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.
Pull Request Overview
This PR fixes an issue where imports used only in computed property names of abstract class members were not being properly elided during compilation. Abstract members don't generate runtime code, so their imports should be removed when they're only used in type-only contexts.
Key Changes:
- Added logic to detect when an identifier is used within a computed property name of an abstract member
- Modified the alias reference marking to skip identifiers in abstract member computed properties, allowing their imports to be elided
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/compiler/checker.ts |
Added isIdentifierInAbstractMemberComputedPropertyName helper function and integrated it into markIdentifierAliasReferenced to prevent marking imports as referenced when used only in abstract member computed properties |
tests/cases/compiler/abstractMethodComputedPropertyElision.ts |
Added compiler test demonstrating the fix with both type-only and normal imports using symbols in abstract computed properties |
tests/baselines/reference/abstractMethodComputedPropertyElision.types |
Baseline file for type checking output of the test case |
tests/baselines/reference/abstractMethodComputedPropertyElision.symbols |
Baseline file for symbol resolution output of the test case |
tests/baselines/reference/abstractMethodComputedPropertyElision.js |
Baseline file showing the compiled JavaScript output where imports are correctly elided |
Comments suppressed due to low confidence (1)
src/compiler/checker.ts:1
- [nitpick] The condition spans multiple lines with inconsistent parentheses grouping. The logical grouping
(isMethodDeclaration || isMethodSignature || isPropertyDeclaration || isPropertySignature)should be fully enclosed in parentheses before the&&operator for clarity.
import {
This comment was marked as resolved.
This comment was marked as resolved.
|
@typescript-bot pack this |
|
Hey @RyanCavanaugh, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
|
@typescript-bot perf test this |
|
@RyanCavanaugh Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
tsserverComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
startupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Fixes #33525