Skip to content
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

fix: prevent stack overflow with attribute changes #2661

Merged
merged 2 commits into from
May 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading