Skip to content

Commit

Permalink
feat: enhance feature initialization (#9479)
Browse files Browse the repository at this point in the history
Previously, feature loading depended on the import order, requiring features to be imported before the component definition. This caused issues in some situations, especially when creating chunks because the imports can be reordered by the build tools.

With this change, we have split the features into two types: library-specific and component-specific. Library-specific features need to be imported before the boot process, otherwise, it can cause serious issues, because the need to re-render all components and manipulate the DOM (including scripts, styles, and meta tags). Component-specific features can now be imported without a specific order, and components that depend on these features will automatically update, enabling the feature on the next rendering of the component.

Fixes: #8175
  • Loading branch information
nnaydenow authored Jul 25, 2024
1 parent 079ee04 commit d55eba8
Show file tree
Hide file tree
Showing 11 changed files with 157 additions and 68 deletions.
54 changes: 54 additions & 0 deletions packages/base/src/FeaturesRegistry.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,21 @@
import EventProvider from "./EventProvider.js";
import type UI5Element from "./UI5Element.js";

abstract class ComponentFeature {
// eslint-disable-next-line @typescript-eslint/no-unused-vars, no-empty-function
constructor(...args: any[]) {}
static define?: () => Promise<void>;
static dependencies?: Array<typeof UI5Element>
}

const features = new Map<string, any>();
const componentFeatures = new Map<string, ComponentFeature>();
const subscribers = new Map<typeof UI5Element, Array<string>>();

const EVENT_NAME = "componentFeatureLoad";
const eventProvider = new EventProvider<undefined, void>();

const featureLoadEventName = (name: string) => `${EVENT_NAME}_${name}`;

const registerFeature = (name: string, feature: object) => {
features.set(name, feature);
Expand All @@ -8,7 +25,44 @@ const getFeature = <T>(name: string): T => {
return features.get(name) as T;
};

const registerComponentFeature = async (name: string, feature: typeof ComponentFeature) => {
await Promise.all(feature.dependencies?.map(dep => dep.define()) || []);
await feature.define?.();

componentFeatures.set(name, feature);
notifyForFeatureLoad(name);
};

const getComponentFeature = <T>(name: string): T => {
return componentFeatures.get(name) as T;
};

const subscribeForFeatureLoad = (name: string, klass: typeof UI5Element, callback: () => void) => {
const subscriber = subscribers.get(klass);
const isSubscribed = subscriber?.includes(name);

if (isSubscribed) {
return;
}

if (!subscriber) {
subscribers.set(klass, [name]);
} else {
subscriber.push(name);
}

eventProvider.attachEvent(featureLoadEventName(name), callback);
};

const notifyForFeatureLoad = (name: string) => {
eventProvider.fireEvent(featureLoadEventName(name), undefined);
};

export {
registerFeature,
getFeature,
registerComponentFeature,
getComponentFeature,
subscribeForFeatureLoad,
ComponentFeature,
};
15 changes: 13 additions & 2 deletions packages/base/src/UI5Element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import type {
} from "./types.js";
import { attachFormElementInternals, setFormValue } from "./features/InputElementsFormSupport.js";
import type { IFormInputElement } from "./features/InputElementsFormSupport.js";
import { subscribeForFeatureLoad } from "./FeaturesRegistry.js";

const DEV_MODE = true;
let autoId = 0;
Expand Down Expand Up @@ -1168,15 +1169,19 @@ abstract class UI5Element extends HTMLElement {
return [];
}

static cacheUniqueDependencies(this: typeof UI5Element): void {
const filtered = this.dependencies.filter((dep, index, deps) => deps.indexOf(dep) === index);
uniqueDependenciesCache.set(this, filtered);
}

/**
* Returns a list of the unique dependencies for this UI5 Web Component
*
* @public
*/
static getUniqueDependencies(this: typeof UI5Element): Array<typeof UI5Element> {
if (!uniqueDependenciesCache.has(this)) {
const filtered = this.dependencies.filter((dep, index, deps) => deps.indexOf(dep) === index);
uniqueDependenciesCache.set(this, filtered);
this.cacheUniqueDependencies();
}

return uniqueDependenciesCache.get(this) || [];
Expand Down Expand Up @@ -1212,6 +1217,12 @@ abstract class UI5Element extends HTMLElement {

const tag = this.getMetadata().getTag();

const features = this.getMetadata().getFeatures();

features.forEach(feature => {
subscribeForFeatureLoad(feature, this, this.cacheUniqueDependencies.bind(this));
});

const definedLocally = isTagRegistered(tag);
const definedGlobally = customElements.get(tag);

Expand Down
9 changes: 9 additions & 0 deletions packages/base/src/UI5ElementMetadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ type Metadata = {
languageAware?: boolean,
formAssociated?: boolean,
shadowRootOptions?: Partial<ShadowRootInit>
features?: Array<string>
};

type State = Record<string, PropertyValue | Array<SlotValue>>;
Expand Down Expand Up @@ -96,6 +97,14 @@ class UI5ElementMetadata {
return this.metadata.tag || "";
}

/**
* Returns the tag of the UI5 Element without the scope
* @private
*/
getFeatures(): Array<string> {
return this.metadata.features || [];
}

/**
* Returns the tag of the UI5 Element
* @public
Expand Down
7 changes: 7 additions & 0 deletions packages/base/src/decorators/customElement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const customElement = (tagNameOrComponentSettings: string | {
fastNavigation?: boolean,
formAssociated?: boolean,
shadowRootOptions?: Partial<ShadowRootInit>,
features?: Array<string>,
} = {}): ClassDecorator => {
return (target: any) => {
if (!Object.prototype.hasOwnProperty.call(target, "metadata")) {
Expand All @@ -38,12 +39,18 @@ const customElement = (tagNameOrComponentSettings: string | {
fastNavigation,
formAssociated,
shadowRootOptions,
features,
} = tagNameOrComponentSettings;

target.metadata.tag = tag;
if (languageAware) {
target.metadata.languageAware = languageAware;
}

if (features) {
target.metadata.features = features;
}

if (themeAware) {
target.metadata.themeAware = themeAware;
}
Expand Down
34 changes: 22 additions & 12 deletions packages/main/src/ColorPalette.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
isUp,
isTabNext,
} from "@ui5/webcomponents-base/dist/Keys.js";
import { getFeature } from "@ui5/webcomponents-base/dist/FeaturesRegistry.js";
import { getComponentFeature } from "@ui5/webcomponents-base/dist/FeaturesRegistry.js";
import ColorPaletteTemplate from "./generated/templates/ColorPaletteTemplate.lit.js";
import ColorPaletteItem from "./ColorPaletteItem.js";
import Button from "./Button.js";
Expand Down Expand Up @@ -73,10 +73,11 @@ type ColorPaletteItemClickEventDetail = {
@customElement({
tag: "ui5-color-palette",
renderer: litRender,
features: ["ColorPaletteMoreColors"],
template: ColorPaletteTemplate,
styles: [ColorPaletteCss, ColorPaletteDialogCss],
get dependencies() {
const colorPaletteMoreColors = getFeature<typeof ColorPaletteMoreColors>("ColorPaletteMoreColors");
const colorPaletteMoreColors = getComponentFeature<typeof ColorPaletteMoreColors>("ColorPaletteMoreColors");
return ([ColorPaletteItem, Button] as Array<typeof UI5Element>).concat(colorPaletteMoreColors ? colorPaletteMoreColors.dependencies : []);
},
})
Expand Down Expand Up @@ -172,19 +173,14 @@ class ColorPalette extends UI5Element {
_itemNavigation: ItemNavigation;
_itemNavigationRecentColors: ItemNavigation;
_recentColors: Array<string>;
moreColorsFeature: ColorPaletteMoreColors | Record<string, any> = {};
moreColorsFeature?: ColorPaletteMoreColors;
_currentlySelected?: ColorPaletteItem;
_shouldFocusRecentColors = false;

static i18nBundle: I18nBundle;

static async onDefine() {
const colorPaletteMoreColors = getFeature<typeof ColorPaletteMoreColors>("ColorPaletteMoreColors");

[ColorPalette.i18nBundle] = await Promise.all([
getI18nBundle("@ui5/webcomponents"),
colorPaletteMoreColors ? colorPaletteMoreColors.init() : Promise.resolve(),
]);
ColorPalette.i18nBundle = await getI18nBundle("@ui5/webcomponents");
}

constructor() {
Expand Down Expand Up @@ -218,17 +214,19 @@ class ColorPalette extends UI5Element {
});

if (this.showMoreColors) {
const ColorPaletteMoreColorsClass = getFeature<typeof ColorPaletteMoreColors>("ColorPaletteMoreColors");
const ColorPaletteMoreColorsClass = getComponentFeature<typeof ColorPaletteMoreColors>("ColorPaletteMoreColors");
if (ColorPaletteMoreColorsClass) {
this.moreColorsFeature = new ColorPaletteMoreColorsClass();
} else {
throw new Error(`You have to import "@ui5/webcomponents/dist/features/ColorPaletteMoreColors.js" module to use the more-colors functionality.`);
}
}

this.onPhone = isPhone();
}

get _effectiveShowMoreColors() {
return !!(this.showMoreColors && this.moreColorsFeature);
}

onAfterRendering() {
if (this._shouldFocusRecentColors && this.hasRecentColors) {
this.recentColorsElements[0].selected = true;
Expand Down Expand Up @@ -528,6 +526,18 @@ class ColorPalette extends UI5Element {
return [...this.effectiveColorItems, ...this.recentColorsElements];
}

get colorPaletteDialogTitle() {
return this.moreColorsFeature?.colorPaletteDialogTitle;
}

get colorPaletteDialogOKButton() {
return this.moreColorsFeature?.colorPaletteDialogOKButton;
}

get colorPaletteCancelButton() {
return this.moreColorsFeature?.colorPaletteCancelButton;
}

/**
* Returns the selected color.
*/
Expand Down
36 changes: 18 additions & 18 deletions packages/main/src/ColorPaletteDialog.hbs
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
{{#if _showMoreColors}}
<ui5-dialog
header-text="{{moreColorsFeature.colorPaletteDialogTitle}}"
>
<div class="ui5-cp-dialog-content">
<ui5-color-picker></ui5-color-picker>
</div>
{{#if _effectiveShowMoreColors}}
<ui5-dialog
header-text="{{colorPaletteDialogTitle}}"
>
<div class="ui5-cp-dialog-content">
<ui5-color-picker></ui5-color-picker>
</div>

<div slot="footer" class="ui5-cp-dialog-footer">
<ui5-button
design="Emphasized"
@click="{{_chooseCustomColor}}"
>{{moreColorsFeature.colorPaletteDialogOKButton}}</ui5-button>
<ui5-button
design="Transparent"
@click="{{_closeDialog}}"
>{{moreColorsFeature.colorPaletteCancelButton}}</ui5-button>
</div>
</ui5-dialog>
<div slot="footer" class="ui5-cp-dialog-footer">
<ui5-button
design="Emphasized"
@click="{{_chooseCustomColor}}"
>{{colorPaletteDialogOKButton}}</ui5-button>
<ui5-button
design="Transparent"
@click="{{_closeDialog}}"
>{{colorPaletteCancelButton}}</ui5-button>
</div>
</ui5-dialog>
{{/if}}
2 changes: 1 addition & 1 deletion packages/main/src/Input.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@

{{> postContent }}

{{#if showSuggestions}}
{{#if _effectiveShowSuggestions}}
<span id="suggestionsText" class="ui5-hidden-text">{{suggestionsText}}</span>
<span id="selectionText" class="ui5-hidden-text" aria-live="polite" role="status"></span>
<span id="suggestionsCount" class="ui5-hidden-text" aria-live="polite">{{availableSuggestionsCount}}</span>
Expand Down
Loading

0 comments on commit d55eba8

Please sign in to comment.