Frontmatter
| title | Overridden methods allow this.callOverridden() |
| author | Dinkh |
| state | Merged |
| createdAt | Feb 1, 2023, 1:17 PM |
| updatedAt | Feb 2, 2023, 11:32 AM |
| closedAt | Feb 2, 2023, 11:32 AM |
| mergedAt | Feb 2, 2023, 11:32 AM |
| branches | dev ← Overrides-patch-2 |
| url | https://github.com/neomjs/neo/pull/3966 |
Merged

tobiu
commented on Feb 1, 2023, 9:09 PM
review in short: looks good for most areas. fine tuning:
callOverridden() => i would cache the regex, e.g. inside a static class field.
@type {{}} will most likely kill the JSDoc parser. not following the way we comment members.
static applyOverrides => using if 2 times followed by a blank char and 2x without (with blank is now inside the new coding guidelines). i would also put cls and items comma separated after overrides.
feel free to apply the changes on your own or ping me if i should do it after the merge. guess writing this comment took more time than applying the fine tuning :P
solving #3965
You can use now
this.callOverridden(methodName, ...arguments);@example
Neo.overrides = { Neo: { component: { Base: { afterSetHeight(value, oldValue) { this.callOverridden('afterSetHeight', ...arguments); } } } } }; export default Neo.overrides;Please make sure to read the Contributing Guidelines:
https://github.com/neomjs/neo/blob/dev/CONTRIBUTING.md
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
If yes, please describe the impact and migration path for existing applications:
The PR fulfills these requirements:
devbranch, not themasterbranchfix #xxx[,#xxx], where "xxx" is the issue number)If adding a new feature, the PR's description includes:
Other information: