-
Notifications
You must be signed in to change notification settings - Fork 7
Use @property()
decorators instead of @state()
#605
Conversation
🦋 Changeset detectedLatest commit: f4129b7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
61d659a
to
c5df4de
Compare
order: [ | ||
'[public-static-properties]', | ||
'[public-@property]', | ||
'[public-@state]', |
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.
Probably worth linting against public @state()
. @ynotdraw: I hear you have a lint rules list somewhere?
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.
haha I do! Added it to my list
c5df4de
to
c6ab0a9
Compare
c6ab0a9
to
03358c5
Compare
order: [ | ||
'[public-static-properties]', | ||
'[public-@property]', | ||
'[public-@state]', |
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.
haha I do! Added it to my list
Pushed an amended commit resolving the conflicts. |
03358c5
to
f4129b7
Compare
🚀 Description
Just a small code quality change.
We sometimes decorate properties with
@state()
when they're only used internally by a subcomponent's parent component. Though@state()
does work the same as@property()
for this purpose,@property()
is the better choice because@state()
is only meant for truly internal properties.From Lit's
@state()
documentation:📋 Checklist
🔬 How to Test
If the tests pass we should be good.
📸 Images/Videos of Functionality
N/A