-
Notifications
You must be signed in to change notification settings - Fork 6
Update to new modifier apis (modify) instead of lifecycles #57
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: gen-4
Are you sure you want to change the base?
Conversation
ember-modifers deprecates lifecycles methods, such as didInstall, didUpdate... replaced by modify. here are some migration guides https://github.com/ember-modifier/ember-modifier/blob/main/MIGRATIONS.md
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.
Wow, thanks! This is awesome!
Please fix two very minor comments and add a CHANGELOG.md
entry (in this project it's maintained by hand).
Note: Node version bump is a breaking change, so bump the major version in the changelog.
@@ -41,9 +43,11 @@ export default class ElementQueryModifier extends Modifier<ModifierArgs> { | |||
sizesHeightDefault: Sizes = SIZES_HEIGHT_DEFAULT; | |||
sizesRatioDefault: Sizes = SIZES_RATIO_DEFAULT; | |||
|
|||
_element?: HTMLElement; // For some reason, this.element is not always available | |||
_element?: HTMLElement; // For some reason, this._element is not always available |
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.
This is wrong, please revert.
|
||
this.didResizeHandler(); | ||
registerDestructor(this, () => this.cleanup()); |
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.
Why this change (arrow function)? In order to remove the lint exception? But we have this exact lint exception used all over the file, so I don't see a point in fixing just one instance.
The linter should serve to the developer, not vice versa. :)
ember-modifers deprecates lifecycles methods, such as didInstall, didUpdate... replaced by modify.
here are some migration guides
https://github.com/ember-modifier/ember-modifier/blob/main/MIGRATIONS.md