Skip to content

Accessors-override-property error allows possibly-correct synthetic properties #42635

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

Closed
wants to merge 2 commits into from

Conversation

sandersn
Copy link
Member

@sandersn sandersn commented Feb 4, 2021

Previously, accessors could not override synthetic properties, such as those created by an intersection, at all if any constituent had the Property symbol flag set. The set of exemptions for abstract or interface-derived properties didn't apply. This PR instead allows those exemptions to apply if they apply to any of the synthetic property's declarations. This allows a mixin pattern used in rush to work.

Fixes #41347

This is the simplest fix -- I'm going to check whether it's worthwhile
to make it stricter next.

Fixes #41347
@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Feb 4, 2021
@rbuckton
Copy link
Contributor

rbuckton commented Feb 4, 2021

Will this change be incompatible with @RyanCavanaugh's investigation into accessors? Having get/set on an interface seems like it would change this behavior.

@sandersn
Copy link
Member Author

sandersn commented Feb 4, 2021

There might be a merge conflict, but Ryan's investigation would just change the function isPropertyAbstractOrFromInterface to isPropertyAbstract (and maybe just into an arrow function at that point). This change just applies the rule across intersections, it doesn't change the rule.

('orthogonal' is overused but it applies here.)

sandersn added a commit that referenced this pull request Feb 10, 2021
Originally from #42635, but this version is simpler.
@sandersn
Copy link
Member Author

#41994 now includes this change, so I'll close this PR.

@sandersn sandersn closed this Feb 10, 2021
@sandersn sandersn deleted the accessors-can-override-synthetic-properties branch February 10, 2021 00:55
sandersn added a commit that referenced this pull request May 31, 2022
* remove too-late fix

* Allow any property from a mapped type

* turn off error for any non-class base

* Also handle synthetic properties more laxly

Originally from #42635, but this version is simpler.

* update baselines

* Update baselines

* createUnionProperty of accessors creates an accessor

Seems simple and doesn't break much. I need to double-check the few test
failures, however.

* Fix computation of write type of accessors

* Calculate property-vs-accessor in existing loop

Instead of looping over the props list 3 more times.

* Undo synthetic accessor change

* Minimise diff
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeScript 4.0 causes unsolvable TS2611 error for properties inherited from a mixin
3 participants