revert: refactor: add simpler componentFocusable util (deprecates `…

…LoadableComponent`) (#9515) **Related Issue:** N/A ## Summary #9362 introduced a utility that aimed to simplify how components wait to be loaded/ready before certain logic. Unfortunately, the behavior differs between `dist` and `components` output targets. This is because the utility assumes components from the `components` output target are not returning promises in lifecycle hooks that would delay initial rendering. See ionic-team/stencil-site#1437 for more info.
Esri · Jun 8, 2024 · 8edeb36 · 8edeb36
1 parent 46c57b4
commit 8edeb36
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 109 deletions.
diff --git a/packages/calcite-components/src/components/accordion-item/accordion-item.tsx b/packages/calcite-components/src/components/accordion-item/accordion-item.tsx
@@ -24,7 +24,12 @@ import {
 import { CSS_UTILITY } from "../../utils/resources";
 import { getIconScale } from "../../utils/component";
 import { FlipContext, Position, Scale, SelectionMode, IconType } from "../interfaces";
-import { componentFocusable } from "../../utils/component";
+import {
+  componentFocusable,
+  LoadableComponent,
+  setComponentLoaded,
+  setUpLoadableComponent,
+} from "../../utils/loadable";
 import { SLOTS, CSS, IDS } from "./resources";
 import { RequestedItem } from "./interfaces";
 
@@ -38,7 +43,7 @@ import { RequestedItem } from "./interfaces";
   styleUrl: "accordion-item.scss",
   shadow: true,
 })
-export class AccordionItem implements ConditionalSlotComponent {
+export class AccordionItem implements ConditionalSlotComponent, LoadableComponent {
   //--------------------------------------------------------------------------
   //
   //  Public Properties
@@ -116,6 +121,14 @@ export class AccordionItem implements ConditionalSlotComponent {
     connectConditionalSlotComponent(this);
   }
 
+  componentWillLoad(): void {
+    setUpLoadableComponent(this);
+  }
+
+  componentDidLoad(): void {
+    setComponentLoaded(this);
+  }
+
   disconnectedCallback(): void {
     disconnectConditionalSlotComponent(this);
   }
@@ -300,7 +313,7 @@ export class AccordionItem implements ConditionalSlotComponent {
   /** Sets focus on the component. */
   @Method()
   async setFocus(): Promise<void> {
-    await componentFocusable(this.el);
+    await componentFocusable(this);
     this.headerEl.focus();
   }
 

diff --git a/packages/calcite-components/src/utils/component.spec.ts b/packages/calcite-components/src/utils/component.spec.ts
@@ -1,7 +1,6 @@
 import { HTMLStencilElement } from "@stencil/core/internal";
 import { html } from "../../support/formatting";
-import * as componentUtils from "./component";
-const { componentFocusable, componentOnReady, getIconScale } = componentUtils;
+import { componentOnReady, getIconScale } from "./component";
 
 describe("getIconScale", () => {
   it('should return "m" when input is "l"', () => {
@@ -19,7 +18,7 @@ describe("componentOnReady", () => {
   let fakeComponent: HTMLElement;
 
   beforeEach(() => {
-    document.body.innerHTML = html`<fake-component></fake-component> `;
+    document.body.innerHTML = html` <fake-component></fake-component> `;
     fakeComponent = document.querySelector<HTMLElement>("fake-component");
 
     const originalRaf = globalThis.requestAnimationFrame;
@@ -50,47 +49,3 @@ describe("componentOnReady", () => {
     expect(requestAnimationFrameSpy).toHaveBeenCalled();
   });
 });
-
-describe("componentFocusable", () => {
-  let componentOnReadyStub: jest.SpyInstance;
-  let fakeComponent: HTMLStencilElement;
-  let forceUpdateSpy: jest.SpyInstance;
-  let requestAnimationFrameSpy: jest.SpyInstance;
-  let componentOnReadyResolver: (el: HTMLStencilElement) => void;
-
-  beforeEach(async () => {
-    document.body.innerHTML = html`<fake-component></fake-component> `;
-    fakeComponent = document.querySelector<HTMLStencilElement>("fake-component");
-
-    const componentOnReadyPromise = new Promise<HTMLStencilElement>(
-      (resolve: (el: HTMLStencilElement) => void) => (componentOnReadyResolver = resolve),
-    );
-    componentOnReadyStub = fakeComponent.componentOnReady = jest.fn(() => componentOnReadyPromise);
-    forceUpdateSpy = jest.spyOn(componentUtils, "forceUpdate").mockImplementation(jest.fn());
-
-    const originalRaf = globalThis.requestAnimationFrame;
-    requestAnimationFrameSpy = jest
-      .spyOn(globalThis, "requestAnimationFrame")
-      .mockImplementation((callback) => originalRaf(callback));
-  });
-
-  afterEach(() => {
-    requestAnimationFrameSpy.mockRestore();
-    forceUpdateSpy.mockRestore();
-  });
-
-  it("should resolve when ready and rendered", async () => {
-    const promise = componentFocusable(fakeComponent);
-    expect(promise).toBeInstanceOf(Promise);
-
-    expect(componentOnReadyStub).toHaveBeenCalled();
-    expect(requestAnimationFrameSpy).not.toHaveBeenCalled();
-    expect(forceUpdateSpy).not.toHaveBeenCalled();
-
-    componentOnReadyResolver(fakeComponent);
-    await promise;
-
-    expect(forceUpdateSpy).toHaveBeenCalled();
-    expect(requestAnimationFrameSpy).toHaveBeenCalled();
-  });
-});
diff --git a/packages/calcite-components/src/utils/component.ts b/packages/calcite-components/src/utils/component.ts
@@ -1,4 +1,3 @@
-import { Build, forceUpdate as stencilForceUpdate } from "@stencil/core";
 import { HTMLStencilElement } from "@stencil/core/internal";
 import { Scale } from "../components/interfaces";
 
@@ -22,40 +21,3 @@ export async function componentOnReady(el: HTMLElement): Promise<void> {
 function isStencilEl(el: HTMLElement): el is HTMLStencilElement {
   return typeof (el as HTMLStencilElement).componentOnReady === "function";
 }
-
-/**
- * Exported for testing purposes only.
- *
- * @internal
- */
-export const forceUpdate = Build.isTesting
-  ? stencilForceUpdate
-  : () => {
-      /* noop */
-    };
-
-/**
- * This helper util can be used to ensuring the component is loaded and rendered by the browser (The "componentDidLoad" Stencil lifecycle method has been called and any internal elements are focusable).
- *
- * A component developer can await this method before proceeding with any logic that requires a component to be loaded first and then an internal element be focused.
- *
- * ```
- * async setFocus(): Promise<void> {
- *   await componentFocusable(this);
- *   this.internalElement?.focus();
- * }
- * ```
- *
- * @param el the component's host element
- * @returns Promise<void>
- */
-export async function componentFocusable(el: HTMLElement): Promise<void> {
-  await componentOnReady(el);
-
-  if (!Build.isBrowser && !Build.isTesting) {
-    return;
-  }
-
-  forceUpdate(el);
-  return new Promise((resolve) => requestAnimationFrame(() => resolve()));
-}
diff --git a/packages/calcite-components/src/utils/dom.spec.ts b/packages/calcite-components/src/utils/dom.spec.ts
@@ -633,7 +633,7 @@ describe("dom", () => {
       // we clobber Stencil's custom Mock document implementation
       const { window: win } = new JSDOM();
 
-      // make window references use JSDOM (which is a subset, hence the type cast)
+      // eslint-disable-next-line no-global-assign -- overriding to make window references use JSDOM (which is a subset, hence the type cast)
       window = win as any as Window & typeof globalThis;
 
       // we define TransitionEvent since JSDOM doesn't support it yet - https://github.com/jsdom/jsdom/issues/1781
@@ -699,7 +699,7 @@ describe("dom", () => {
       // we clobber Stencil's custom Mock document implementation
       const { window: win } = new JSDOM();
 
-      // make window references use JSDOM (which is a subset, hence the type cast)
+      // eslint-disable-next-line no-global-assign -- overriding to make window references use JSDOM (which is a subset, hence the type cast)
       window = win as any as Window & typeof globalThis;
 
       // we define AnimationEvent since JSDOM doesn't support it yet -

diff --git a/packages/calcite-components/src/utils/loadable.ts b/packages/calcite-components/src/utils/loadable.ts
@@ -1,4 +1,4 @@
-import { componentFocusable as componentFocusableReplacement } from "./component";
+import { Build, forceUpdate } from "@stencil/core";
 
 /**
  * This helper adds support for knowing when a component has been loaded.
@@ -37,30 +37,23 @@ import { componentFocusable as componentFocusableReplacement } from "./component
  *    await componentLoaded(this);
  *  }
  * ```
- *
- * @deprecated use `componentOnReady` from `components.ts` instead.
  */
 export interface LoadableComponent {
-  /**
-   * The host element.
-   */
-  el: HTMLElement;
-
   /**
    * Stencil lifecycle method.
    * https://stenciljs.com/docs/component-lifecycle#componentwillload
    *
    * Called once just after the component is first connected to the DOM. Since this method is only called once, it's a good place to load data asynchronously and to setup the state without triggering extra re-renders.
    */
-  componentWillLoad?: () => Promise<void> | void;
+  componentWillLoad: () => Promise<void> | void;
 
   /**
    * Stencil lifecycle method.
    * https://stenciljs.com/docs/component-lifecycle#componentdidload
    *
    * Called once just after the component is fully loaded and the first render() occurs.
    */
-  componentDidLoad?: () => Promise<void> | void;
+  componentDidLoad: () => Promise<void> | void;
 }
 
 const resolveMap = new WeakMap<LoadableComponent, (value: void | PromiseLike<void>) => void>();
@@ -79,8 +72,6 @@ const promiseMap = new WeakMap<LoadableComponent, Promise<void>>();
  * ```
  *
  * @param component
- *
- * @deprecated use `componentOnReady` from `components.ts` instead.
  */
 export function setUpLoadableComponent(component: LoadableComponent): void {
   promiseMap.set(component, new Promise((resolve) => resolveMap.set(component, resolve)));
@@ -98,8 +89,6 @@ export function setUpLoadableComponent(component: LoadableComponent): void {
  * ```
  *
  * @param component
- *
- * @deprecated use `componentOnReady` from `components.ts` instead.
  */
 export function setComponentLoaded(component: LoadableComponent): void {
   resolveMap.get(component)();
@@ -120,8 +109,6 @@ export function setComponentLoaded(component: LoadableComponent): void {
  *
  * @param component
  * @returns Promise<void>
- *
- * @deprecated use `componentOnReady` from `components.ts` instead.
  */
 export function componentLoaded(component: LoadableComponent): Promise<void> {
   return promiseMap.get(component);
@@ -143,9 +130,14 @@ export function componentLoaded(component: LoadableComponent): Promise<void> {
  *
  * @param component
  * @returns Promise<void>
- *
- * @deprecated use `componentFocusable` from `components.ts` instead.
  */
 export async function componentFocusable(component: LoadableComponent): Promise<void> {
-  await componentFocusableReplacement(component.el);
+  await componentLoaded(component);
+
+  if (!Build.isBrowser) {
+    return;
+  }
+
+  forceUpdate(component);
+  return new Promise((resolve) => requestAnimationFrame(() => resolve()));
 }