+
Skip to content
This repository was archived by the owner on Oct 14, 2025. It is now read-only.

Conversation

clintcs
Copy link
Collaborator

@clintcs clintcs commented Jan 15, 2025

🚀 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:

Properties declared [using @state()] must not be used from HTML or HTML templating systems, they're solely for properties internal to the element. These properties may be renamed by optimization tools like closure compiler.

📋 Checklist

🔬 How to Test

If the tests pass we should be good.

📸 Images/Videos of Functionality

N/A

Copy link

changeset-bot bot commented Jan 15, 2025

🦋 Changeset detected

Latest commit: f4129b7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@crowdstrike/glide-core Patch

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

Copy link
Contributor

@clintcs clintcs force-pushed the use-property-decorator-instead-of-state branch from 61d659a to c5df4de Compare January 15, 2025 16:23
order: [
'[public-static-properties]',
'[public-@property]',
'[public-@state]',
Copy link
Collaborator Author

@clintcs clintcs Jan 15, 2025

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?

Copy link
Collaborator

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

@clintcs clintcs force-pushed the use-property-decorator-instead-of-state branch from c5df4de to c6ab0a9 Compare January 15, 2025 16:26
@clintcs clintcs marked this pull request as ready for review January 15, 2025 16:28
@clintcs clintcs force-pushed the use-property-decorator-instead-of-state branch from c6ab0a9 to 03358c5 Compare January 15, 2025 16:34
order: [
'[public-static-properties]',
'[public-@property]',
'[public-@state]',
Copy link
Collaborator

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

@clintcs
Copy link
Collaborator Author

clintcs commented Jan 15, 2025

Pushed an amended commit resolving the conflicts.

@clintcs clintcs force-pushed the use-property-decorator-instead-of-state branch from 03358c5 to f4129b7 Compare January 15, 2025 19:05
@clintcs clintcs merged commit 32c8aa0 into main Jan 15, 2025
7 checks passed
@clintcs clintcs deleted the use-property-decorator-instead-of-state branch January 15, 2025 19:16
@github-actions github-actions bot mentioned this pull request Jan 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载