Icu 16534 refactor usage of tracked using this args #2711
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| if (DYNAMIC_CREDENTIAL_FIELDS.some((field) => this[field])) { | ||
| this.#credentialType = TYPE_CREDENTIAL_DYNAMIC; | ||
| return TYPE_CREDENTIAL_DYNAMIC; |
There was a problem hiding this comment.
None blocking comment:
The previous implementation checks if this.credentialType exists.
If it doesn't, it sets a value.
At the end, it always returns this.credentialType
The new implementation checks if this.credentialType exists.
If it doesnt', returns a value. BUT, and here is my questions, never sets the credentialType @Tracked property. I have nothing against, I would like to learn Why we change the approach on this?
If it does, returns the this.credentialType
There was a problem hiding this comment.
Great question, this getter shouldn't set the value of credentialType once it was changed to be tracked because we get this error. It also doesn't need to set the value for credentialType because the getter takes care of returning the correct type on initial load (when the private variable credentialType is undefined).
|
Looking great so far!! left a couple of comments and will wait on other feedback too 😉 |
| // If the role_arn is empty, then the credential type should be static | ||
| if (!storageBucket.role_arn) { | ||
| storageBucket.credentialType = TYPE_CREDENTIAL_STATIC; | ||
| } |
There was a problem hiding this comment.
if user has an existing storage bucket with static credential type and edits type to dynamic credential without entering a role_arn value then on save it will show type change to 'dynamic' even though its still static. So this change was added so that type stays as 'static'. We already do the same for host-catalogs.
| // If the role_arn is empty, then the credential type should be static | ||
| if (!storageBucket.role_arn) { | ||
| storageBucket.credentialType = TYPE_CREDENTIAL_STATIC; | ||
| } |
🎟️ Jira ticket
Description
This refactor is in preparation for ember upgrade that will update eslint which comes with some new rules. One of them is disallowing usage of
this.argswith@tracked. Our usage was limited to initializing tracked variables so that was refactored to use getters instead to initialize.How to Test
Checklist