Skip to content

Commit

Permalink
fix: prevent stack overflow with attribute changes
Browse files Browse the repository at this point in the history
Signed-off-by: Jeremias Peier <[email protected]>
  • Loading branch information
jeripeierSBB committed May 15, 2024
1 parent ec4fdc8 commit e7f5751
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 5 deletions.
24 changes: 22 additions & 2 deletions src/components/core/base-elements/button-base-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@ export abstract class SbbButtonBaseElement extends SbbActionBaseElement {
/** The type attribute to use for the button. */
@property() public type: SbbButtonType = 'button';

/** The name of the button element. */
/**
* The name of the button element.
*
* @description Developer note: In this case updating the attribute must be synchronous.
* Due to this it is implemented as a getter/setter and the attributeChangedCallback() handles the diff check.
*/
@property()
public set name(name: string) {
this.setAttribute('name', `${name}`);
Expand All @@ -28,7 +33,12 @@ export abstract class SbbButtonBaseElement extends SbbActionBaseElement {
return this.getAttribute('name') ?? '';
}

/** The value of the button element. */
/**
* The value of the button element.
*
* @description Developer note: In this case updating the attribute must be synchronous.
* Due to this it is implemented as a getter/setter and the attributeChangedCallback() handles the diff check.
*/
@property()
public set value(value: string) {
this.setAttribute('value', `${value}`);
Expand Down Expand Up @@ -128,4 +138,14 @@ export abstract class SbbButtonBaseElement extends SbbActionBaseElement {
);
}
}

public override attributeChangedCallback(
name: string,
old: string | null,
value: string | null,
): void {
if (!['name', 'value'].includes(name) || old !== value) {
super.attributeChangedCallback(name, old, value);
}
}
}
17 changes: 16 additions & 1 deletion src/components/core/mixins/form-associated-mixin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,12 @@ export const SbbFormAssociatedMixin = <T extends Constructor<LitElement>>(
return this.internals.form;
}

/** Name of the form element. Will be read from name attribute. */
/**
* Name of the form element. Will be read from name attribute.
*
* @description Developer note: In this case updating the attribute must be synchronous.
* Due to this it is implemented as a getter/setter and the attributeChangedCallback() handles the diff check.
*/
@property()
public set name(name: string) {
this.setAttribute('name', `${name}`);
Expand Down Expand Up @@ -112,6 +117,16 @@ export const SbbFormAssociatedMixin = <T extends Constructor<LitElement>>(
/** Whenever a surrounding form or fieldset is changing its disabled state. */
@state() protected formDisabled: boolean = false;

public override attributeChangedCallback(
name: string,
old: string | null,
value: string | null,
): void {
if (name !== 'name' || old !== value) {
super.attributeChangedCallback(name, old, value);
}
}

/**
* Returns true if internals target element has no validity problems; false otherwise.
* Fires an invalid event at the element in the latter case.
Expand Down
19 changes: 17 additions & 2 deletions src/components/option/option/option.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,12 @@ export class SbbOptionElement extends SbbDisabledMixin(SbbIconNameMixin(LitEleme
optionSelected: 'optionSelected',
} as const;

/** Value of the option. */
/**
* Value of the option.
*
* @description Developer note: In this case updating the attribute must be synchronous.
* Due to this it is implemented as a getter/setter and the attributeChangedCallback() handles the diff check.
*/
@property()
public set value(value: string) {
this.setAttribute('value', `${value}`);
Expand Down Expand Up @@ -92,7 +97,7 @@ export class SbbOptionElement extends SbbDisabledMixin(SbbIconNameMixin(LitEleme
SbbOptionElement.events.optionSelected,
);

/** Wheter to apply the negative styling */
/** Whether to apply the negative styling */
@state() private _negative = false;

/** Whether the component must be set disabled due disabled attribute on sbb-checkbox-group. */
Expand Down Expand Up @@ -137,6 +142,16 @@ export class SbbOptionElement extends SbbDisabledMixin(SbbIconNameMixin(LitEleme
new SbbSlotStateController(this);
}

public override attributeChangedCallback(
name: string,
old: string | null,
value: string | null,
): void {
if (name !== 'value' || old !== value) {
super.attributeChangedCallback(name, old, value);
}
}

/**
* Highlight the label of the option
* @param value the highlighted portion of the label
Expand Down

0 comments on commit e7f5751

Please sign in to comment.