feat(componentDidUnload): use disconnectedCallback instead of compone…

…ntDidUnload

When Stencil is used within other frameworks, elements may be reused, making it impossible for componentDidUnload to be accurate 100% of the time if it is disconnected more than once. Instead, disconnectedCallback() is the preferred way to always know if a component was disconnected from the DOM.

Note that the runtime still works for any collections that have been built with componentDidUnload(). However, updates to Stencil 2 will require it's changed to disconnectedCallback().

STYLE_GUIDE.md

@@ -209,13 +209,10 @@ export class Something {
    * Ordered by their natural call order, for example
    * WillLoad should go before DidLoad.
    */
+  connectedCallback() {}
   componentWillLoad() {}
   componentDidLoad() {}
-  componentWillEnter() {}
-  componentDidEnter() {}
-  componentWillLeave() {}
-  componentDidLeave() {}
-  componentDidUnload() {}
+  disconnectedCallbac() {}
 
   /**
    * 8. Listeners

src/compiler/transformers/decorators-to-static/convert-decorators.ts

@@ -5,7 +5,7 @@ import { elementDecoratorsToStatic } from './element-decorator';
 import { eventDecoratorsToStatic } from './event-decorator';
 import { listenDecoratorsToStatic } from './listen-decorator';
 import { isDecoratorNamed } from './decorator-utils';
-import { methodDecoratorsToStatic } from './method-decorator';
+import { methodDecoratorsToStatic, validateMethods } from './method-decorator';
 import { propDecoratorsToStatic } from './prop-decorator';
 import { stateDecoratorsToStatic } from './state-decorator';
 import { watchDecoratorsToStatic } from './watch-decorator';
@@ -36,8 +36,9 @@ export const visitClassDeclaration = (config: d.Config, diagnostics: d.Diagnosti
     return classNode;
   }
 
-  const decoratedMembers = classNode.members.filter(member => Array.isArray(member.decorators) && member.decorators.length > 0);
-  const newMembers = removeStencilDecorators(Array.from(classNode.members));
+  const classMembers = classNode.members;
+  const decoratedMembers = classMembers.filter(member => Array.isArray(member.decorators) && member.decorators.length > 0);
+  const newMembers = removeStencilDecorators(Array.from(classMembers));
 
   // parser component decorator (Component)
   componentDecoratorToStatic(config, typeChecker, diagnostics, classNode, newMembers, componentDecorator);
@@ -54,6 +55,8 @@ export const visitClassDeclaration = (config: d.Config, diagnostics: d.Diagnosti
     listenDecoratorsToStatic(diagnostics, decoratedMembers, newMembers);
   }
 
+  validateMethods(diagnostics, classMembers);
+
   return ts.updateClassDeclaration(
     classNode,
     removeDecorators(classNode, CLASS_DECORATORS_TO_REMOVE),

src/compiler/transformers/decorators-to-static/method-decorator.ts

@@ -89,3 +89,14 @@ const parseMethodDecorator = (config: d.Config, diagnostics: d.Diagnostic[], tsS
 const isTypePromise = (typeStr: string) => {
   return /^Promise<.+>$/.test(typeStr);
 };
+
+export const validateMethods = (diagnostics: d.Diagnostic[], members: ts.NodeArray<ts.ClassElement>) => {
+  members.filter(ts.isMethodDeclaration).map(method => {
+    if (method.name.getText() === 'componentDidUnload') {
+      const err = buildError(diagnostics);
+      err.header = `Replace "componentDidUnload()" with "disconnectedCallback()"`;
+      err.messageText = `The "componentDidUnload()" method was removed in Stencil 2. Please use the "disconnectedCallbac()" method instead.`;
+      augmentDiagnosticWithNode(err, method.name);
+    }
+  });
+};

src/compiler/transformers/static-to-meta/component.ts

@@ -41,7 +41,6 @@ export const parseStaticComponentMeta = (
   const encapsulation = parseStaticEncapsulation(staticMembers);
 
   const cmp: d.ComponentCompilerMeta = {
-    isLegacy: false,
     tagName: tagName,
     excludeFromCollection: moduleFile.excludeFromCollection,
     isCollectionDependency,

src/compiler/transpile/run-program.ts

@@ -1,14 +1,15 @@
-import * as d from '../../declarations';
-import { basename, join, relative } from 'path';
-import { buildError, loadTypeScriptDiagnostics, normalizePath } from '@utils';
+import type * as d from '../../declarations';
+import { basename, join } from 'path';
 import { convertDecoratorsToStatic } from '../transformers/decorators-to-static/convert-decorators';
 import { generateAppTypes } from '../types/generate-app-types';
 import { getComponentsFromModules, isOutputTargetDistTypes } from '../output-targets/output-utils';
+import { loadTypeScriptDiagnostics, normalizePath } from '@utils';
 import { resolveComponentDependencies } from '../entries/resolve-component-dependencies';
+import type ts from 'typescript';
 import { updateComponentBuildConditionals } from '../app-core/app-data';
 import { updateModule } from '../transformers/static-to-meta/parse-static';
-import ts from 'typescript';
 import { updateStencilTypesImports } from '../types/stencil-types';
+import { validateTranspiledComponents } from './validate-components';
 
 export const runTsProgram = async (config: d.Config, compilerCtx: d.CompilerCtx, buildCtx: d.BuildCtx, tsBuilder: ts.BuilderProgram) => {
   const tsSyntactic = loadTypeScriptDiagnostics(tsBuilder.getSyntacticDiagnostics());
@@ -54,7 +55,7 @@ export const runTsProgram = async (config: d.Config, compilerCtx: d.CompilerCtx,
   updateComponentBuildConditionals(compilerCtx.moduleMap, buildCtx.components);
   resolveComponentDependencies(buildCtx.components);
 
-  validateUniqueTagNames(config, buildCtx);
+  validateTranspiledComponents(config, buildCtx);
 
   if (buildCtx.hasError) {
     return false;
@@ -82,18 +83,6 @@ export const runTsProgram = async (config: d.Config, compilerCtx: d.CompilerCtx,
   return false;
 };
 
-const validateUniqueTagNames = (config: d.Config, buildCtx: d.BuildCtx) => {
-  buildCtx.components.forEach(cmp => {
-    const tagName = cmp.tagName;
-    const cmpsWithTagName = buildCtx.components.filter(c => c.tagName === tagName);
-    if (cmpsWithTagName.length > 1) {
-      const err = buildError(buildCtx.diagnostics);
-      err.header = `Component Tag Name "${tagName}" Must Be Unique`;
-      err.messageText = `Please update the components so "${tagName}" is only used once: ${cmpsWithTagName.map(c => relative(config.rootDir, c.sourceFilePath)).join(' ')}`;
-    }
-  });
-};
-
 const getRelativeDts = (config: d.Config, srcPath: string, emitDtsPath: string) => {
   const parts: string[] = [];
   srcPath = normalizePath(srcPath);

src/compiler/transpile/validate-components.ts

@@ -0,0 +1,19 @@
+import type * as d from '../../declarations';
+import { buildError } from '@utils';
+import { relative } from 'path';
+
+export const validateTranspiledComponents = (config: d.Config, buildCtx: d.BuildCtx) => {
+  for (const cmp of buildCtx.components) {
+    validateUniqueTagNames(config, buildCtx, cmp);
+  }
+};
+
+const validateUniqueTagNames = (config: d.Config, buildCtx: d.BuildCtx, cmp: d.ComponentCompilerMeta) => {
+  const tagName = cmp.tagName;
+  const cmpsWithTagName = buildCtx.components.filter(c => c.tagName === tagName);
+  if (cmpsWithTagName.length > 1) {
+    const err = buildError(buildCtx.diagnostics);
+    err.header = `Component Tag Name "${tagName}" Must Be Unique`;
+    err.messageText = `Please update the components so "${tagName}" is only used once: ${cmpsWithTagName.map(c => relative(config.rootDir, c.sourceFilePath)).join(' ')}`;
+  }
+};

src/compiler/types/generate-app-types.ts

@@ -45,7 +45,6 @@ const generateComponentTypesFile = (config: d.Config, buildCtx: d.BuildCtx, inte
   let typeImportData: d.TypesImportData = {};
   const c: string[] = [];
   const allTypes = new Map<string, number>();
-  const needsJSXElementHack = buildCtx.components.some(cmp => cmp.isLegacy);
   const components = buildCtx.components.filter(m => !m.isCollectionDependency);
 
   const modules: d.TypesModule[] = components.map(cmp => {
@@ -83,13 +82,6 @@ const generateComponentTypesFile = (config: d.Config, buildCtx: d.BuildCtx, inte
 
   c.push(`declare global {`);
 
-  if (needsJSXElementHack) {
-    c.push(`    // Adding a global JSX for backcompatibility with legacy dependencies`);
-    c.push(`    export namespace JSX {`);
-    c.push(`        export interface Element {}`);
-    c.push(`    }`);
-  }
-
   c.push(...modules.map(m => m.element));
 
   c.push(`        interface HTMLElementTagNameMap {`);

src/declarations/stencil-core/index.ts

@@ -4,7 +4,6 @@ export {
   Component,
   ComponentOptions,
   ComponentDidLoad,
-  ComponentDidUnload,
   ComponentDidUpdate,
   ComponentInterface,
   ComponentWillLoad,

src/declarations/stencil-private.ts

@@ -693,7 +693,6 @@ export interface ComponentCompilerMeta extends ComponentCompilerFeatures {
   shadowDelegatesFocus: boolean;
   excludeFromCollection: boolean;
   isCollectionDependency: boolean;
-  isLegacy: boolean;
   docs: CompilerJsDoc;
   jsFilePath: string;
   listeners: ComponentCompilerListener[];

src/declarations/stencil-public-runtime.ts

@@ -363,14 +363,6 @@ export interface ComponentDidUpdate {
   componentDidUpdate(): void;
 }
 
-export interface ComponentDidUnload {
-  /**
-   * The component did unload and the element
-   * will be destroyed.
-   */
-  componentDidUnload(): void;
-}
-
 export interface ComponentInterface {
   connectedCallback?(): void;
   disconnectedCallback?(): void;

test/karma/test-app/dom-reattach/cmp-root.tsx

@@ -1,13 +1,12 @@
 import { Component, h, Host, Prop } from '@stencil/core';
 
 @Component({
-  tag: 'dom-reattach'
+  tag: 'dom-reattach',
 })
 export class DomReattach {
-
-  @Prop({mutable: true}) willLoad = 0;
-  @Prop({mutable: true}) didLoad = 0;
-  @Prop({mutable: true}) didUnload = 0;
+  @Prop({ mutable: true }) willLoad = 0;
+  @Prop({ mutable: true }) didLoad = 0;
+  @Prop({ mutable: true }) didUnload = 0;
 
   componentWillLoad() {
     this.willLoad++;
@@ -17,7 +16,7 @@ export class DomReattach {
     this.didLoad++;
   }
 
-  componentDidUnload() {
+  disconnectedCallback() {
     this.didUnload++;
   }
 
@@ -26,8 +25,8 @@ export class DomReattach {
       <Host>
         <p>componentWillLoad: {this.willLoad}</p>
         <p>componentDidLoad: {this.didLoad}</p>
-        <p>componentDidUnload: {this.didUnload}</p>
+        <p>disconnectedCallback: {this.didUnload}</p>
       </Host>
-    )
+    );
   }
 }

test/karma/test-app/lifecycle-unload/cmp-a.tsx

@@ -2,18 +2,17 @@ import { Component, Element, h } from '@stencil/core';
 
 @Component({
   tag: 'lifecycle-unload-a',
-  shadow: true
+  shadow: true,
 })
 export class LifecycleUnloadA {
-
   @Element() el!: HTMLElement;
   results?: HTMLDivElement | null;
 
   componentDidLoad() {
     this.results = this.el.ownerDocument!.body.querySelector('#lifecycle-unload-results') as HTMLDivElement;
   }
 
-  componentDidUnload() {
+  disconnectedCallback() {
     const elm = document.createElement('div');
     elm.textContent = 'cmp-a unload';
     this.results!.appendChild(elm);

test/karma/test-app/lifecycle-unload/cmp-b.tsx

@@ -2,18 +2,17 @@ import { Component, Element, h } from '@stencil/core';
 
 @Component({
   tag: 'lifecycle-unload-b',
-  shadow: true
+  shadow: true,
 })
 export class LifecycleUnloadB {
-
   @Element() el!: HTMLElement;
   results?: HTMLDivElement;
 
   componentDidLoad() {
     this.results = this.el.ownerDocument!.body.querySelector('#lifecycle-unload-results') as HTMLDivElement;
   }
 
-  componentDidUnload() {
+  disconnectedCallback() {
     const elm = document.createElement('div');
     elm.textContent = 'cmp-b unload';
     this.results!.appendChild(elm);
@@ -22,8 +21,10 @@ export class LifecycleUnloadB {
   render() {
     return [
       <article>cmp-b - top</article>,
-      <section><slot/></section>,
-      <nav>cmp-b - bottom</nav>
+      <section>
+        <slot />
+      </section>,
+      <nav>cmp-b - bottom</nav>,
     ];
   }
 }