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

Keep accessors as accessors in emitted anonymous class declarations #60853

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Andarist
Copy link
Contributor

@Andarist Andarist commented Dec 25, 2024

fixes #60672
fixes #44938
fixes #58790

@typescript-bot typescript-bot added For Backlog Bug PRs that fix a backlog bug For Milestone Bug PRs that fix a bug with a specific milestone labels Dec 25, 2024
@Andarist Andarist force-pushed the keep-accessors-as-accessors-on-emitted-anonymous-class-declarations branch from d466c50 to 1baa828 Compare January 1, 2025 19:11
@@ -14939,6 +14966,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}
if (!singleProp) {
singleProp = prop;
propFlags = (prop.flags & SymbolFlags.Accessor) || SymbolFlags.Property;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the accessor case should be handled through checkFlags. I'm worried that it would require looking carefully at all call sites that handle accessors to cover for this new case - but maybe I'm worried about it prematurely. Anyway, let me know what you think about it ;)

Comment on lines +14992 to +14993
// classes created by mixins are represented as intersections and overriding a property in a derived class redefines it completely at runtime
// so a get accessor can't be merged with a set accessor in a base class, for that reason the accessor flags are only used when they are the same in all consistuents
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps if I'd use a new CheckFlags.SyntheticAccessor that wouldn't really matter as much? I'm not sure if it's even that important of a rule to begin with, I just erred on the safer side of things and maintained the current behavior for this ambiguous case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Backlog Bug PRs that fix a backlog bug For Milestone Bug PRs that fix a bug with a specific milestone
Projects
Status: Not started
3 participants