refactor(core): remove $templateQRL (QwikDev#49)

* refactor(core): remove `$templateQRL` `$templateQRL` was used to verify that the component retrieved was of the correct type. We plan to have all QRLs to be generated by the tooling and so this kind of verification is not necessary. In addition this prevents progress of the tooling. There are plans to re-do the way this code works to remove the injection pattern. * fixup! refactor(core): remove `$templateQRL`
willkuerlich · Aug 4, 2021 · 9a0a49c · 9a0a49c
1 parent 8572753
commit 9a0a49c
Show file tree

Hide file tree

Showing 16 changed files with 46 additions and 82 deletions.
diff --git a/integration/hello_server/Greeter_component.ts b/integration/hello_server/Greeter_component.ts
@@ -7,7 +7,7 @@
  */
 
 import type { GreeterProps } from './Greeter';
-import { Component, QRL } from '@builder.io/qwik';
+import { Component } from '@builder.io/qwik';
 
 /**
  * @fileoverview
@@ -32,7 +32,6 @@ export interface GreeterState {
  * non-serializable objects. Component has shared behavior.
  */
 export class GreeterComponent extends Component<GreeterProps, GreeterState> {
-  static $templateQRL = QRL`./Greeter_template`;
   // Inherited properties from `Component`
   // $host: Element;
   // $state: GreeterState;

diff --git a/integration/todo/ui/Header_component.ts b/integration/todo/ui/Header_component.ts
@@ -6,15 +6,14 @@
  * found in the LICENSE file at https://github.com/BuilderIO/qwik/blob/main/LICENSE
  */
 
-import { Component, QRL } from '@builder.io/qwik';
+import { Component } from '@builder.io/qwik';
 import type { HeaderProps } from './Header';
 
 interface HeaderState {
   text: string;
 }
 
 export class HeaderComponent extends Component<HeaderProps, HeaderState> {
-  static $templateQRL = QRL`ui:/Header_template`;
   $newState() {
     return { text: '' };
   }

diff --git a/integration/todo/ui/Item_component.ts b/integration/todo/ui/Item_component.ts
@@ -6,14 +6,12 @@
  * found in the LICENSE file at https://github.com/BuilderIO/qwik/blob/main/LICENSE
  */
 
-import { Component, QRL } from '@builder.io/qwik';
+import { Component } from '@builder.io/qwik';
 import type { ItemProps } from './Item';
 
 interface ItemState {}
 
 export class ItemComponent extends Component<ItemProps, ItemState> {
-  static $templateQRL = QRL`ui:/Item_template`;
-
   editing = false;
   $newState() {
     return {};

diff --git a/src/core/api.md b/src/core/api.md
@@ -10,13 +10,11 @@ export class Component<PROPS, STATE> {
     $init(): Promise<void> | void;
     // (undocumented)
     static $new<COMP extends Component<any, any>>(this: {
-        $templateQRL: QRL;
         new (...args: any[]): COMP;
     }, hostElement: Element): Promise<COMP>;
     $newState(props: PROPS): Promise<STATE> | STATE;
     $props: PROPS;
     $state: STATE;
-    static $templateQRL: QRL;
     constructor(hostElement: Element, props: PROPS, state: STATE | null);
 }
 
@@ -28,8 +26,6 @@ export type ComponentChildren = ComponentChild[] | ComponentChild;
 
 // @public
 export interface ComponentConstructor<COMP extends Component<any, any>> {
-    // (undocumented)
-    $templateQRL: QRL;
     // (undocumented)
     new (hostElement: Element, props: ComponentPropsOf<COMP>, state: ComponentStateOf<COMP> | null): COMP;
 }
@@ -251,7 +247,6 @@ export interface InjectedFunction<SELF, ARGS extends any[], REST extends any[],
 // @public
 export function injectEventHandler<SELF, ARGS extends any[], RET>(...args: [
     {
-        $templateQRL: QRL;
         new (hostElement: Element, props: any, state: any): SELF;
     } | null,
     ...ARGS,

diff --git a/src/core/component/component.ts b/src/core/component/component.ts
@@ -6,7 +6,6 @@
  * found in the LICENSE file at https://github.com/BuilderIO/qwik/blob/main/LICENSE
  */
 
-import type { QRL } from '../import/qrl';
 import { QError, qError } from '../error/error';
 import { AttributeMarker } from '../util/markers';
 import { getInjector } from '../injector/element_injector';
@@ -35,14 +34,8 @@ import { getInjector } from '../injector/element_injector';
  * @public
  */
 export class Component<PROPS, STATE> {
-  /**
-   * Pointer to template to verify that the component is attached to the right DOM location.
-   */
-  static $templateQRL: QRL = null!;
-
   static $new<COMP extends Component<any, any>>(
     this: {
-      $templateQRL: QRL;
       new (...args: any[]): COMP;
     },
     hostElement: Element
@@ -51,11 +44,6 @@ export class Component<PROPS, STATE> {
     const componentConstructor = this as any as ComponentConstructor<COMP>;
     const componentTemplate = hostElement.getAttribute(AttributeMarker.ComponentTemplate);
     if (!componentTemplate) {
-      hostElement.setAttribute(
-        AttributeMarker.ComponentTemplate,
-        componentConstructor.$templateQRL as any
-      );
-    } else if (componentTemplate !== (componentConstructor.$templateQRL as any)) {
       // TODO: Needs tests for error condition for attaching component to element  which already has a component
       throw new Error('Write proper error');
     }
@@ -188,7 +176,6 @@ export type ComponentPropsOf<SERVICE extends Component<any, any>> = SERVICE exte
  * @public
  */
 export interface ComponentConstructor<COMP extends Component<any, any>> {
-  $templateQRL: QRL;
   new (
     hostElement: Element,
     props: ComponentPropsOf<COMP>,
@@ -202,5 +189,5 @@ export interface ComponentConstructor<COMP extends Component<any, any>> {
  * @internal
  */
 export function isComponent(object: any): object is Component<any, any> {
-  return typeof object?.constructor?.$templateQRL === 'string';
+  return object instanceof Component;
 }
diff --git a/src/core/component/component.unit.ts b/src/core/component/component.unit.ts
@@ -7,18 +7,15 @@
  */
 
 import { ComponentFixture } from '@builder.io/qwik/testing';
-import { GreeterComponent } from '../util/test_component_fixture';
+import { GreeterComponent, GreeterComponentTemplate } from '../util/test_component_fixture';
 import { AttributeMarker } from '../util/markers';
 import { Component } from './component';
 import { injectMethod } from '../injector/inject';
 
 describe('component', () => {
   it('should declare a component', async () => {
     const fixture = new ComponentFixture();
-    fixture.host.setAttribute(
-      AttributeMarker.ComponentTemplate,
-      String(GreeterComponent.$templateQRL)
-    );
+    fixture.host.setAttribute(AttributeMarker.ComponentTemplate, String(GreeterComponentTemplate));
     fixture.host.setAttribute('salutation', 'Hello');
     fixture.host.setAttribute('name', 'World');
     const greeter = await fixture.injector.getComponent(GreeterComponent);

diff --git a/src/core/error/error.ts b/src/core/error/error.ts
@@ -53,7 +53,6 @@ export const enum QError {
   Component_noProperty_propName_props_host = 404,
   Component_notFound_component = 405,
   Component_doesNotMatch_component_actual = 406,
-  Component_missingTemplateQRL_component = 407,
   Component_noState_component_props = 408,
   // Provider 500-599
   Provider_unrecognizedFormat_value = 500,
@@ -165,9 +164,7 @@ function codeToText(code: QError): string {
       "Property '{}' not found in '{}' on component '{}'.",
     [QError.Component_notFound_component]: "Unable to find '{}' component.",
     [QError.Component_doesNotMatch_component_actual]:
-      "Requesting component '{}' does not match existing component '{}'. Verify that the two components have distinct '$templateQRL's.",
-    [QError.Component_missingTemplateQRL_component]:
-      "Expecting Component '{}' to have static '$templateQRL' property, but none was found.",
+      "Requesting component type '{}' does not match existing component instance '{}'.",
     [QError.Component_noState_component_props]:
       "Unable to create state for component '{}' with props '{}' because no state found and '$newState()' method was not defined on component.",
     //////////////

diff --git a/src/core/event/inject_event_handler.ts b/src/core/event/inject_event_handler.ts
@@ -6,7 +6,6 @@
  * found in the LICENSE file at https://github.com/BuilderIO/qwik/blob/main/LICENSE
  */
 
-import type { QRL } from '../import/qrl';
 import type { InjectedFunction, ProviderReturns } from '../injector/types';
 import { qDev } from '../util/qdev';
 import { EventInjector } from './event_injector';
@@ -41,7 +40,6 @@ import type { EventHandler } from './types';
 export function injectEventHandler<SELF, ARGS extends any[], RET>(
   ...args: [
     {
-      $templateQRL: QRL;
       new (hostElement: Element, props: any, state: any): SELF;
     } | null,
     ...ARGS,

diff --git a/src/core/event/inject_event_handler.unit.ts b/src/core/event/inject_event_handler.unit.ts
@@ -21,7 +21,7 @@ describe('injectEventHandler', () => {
   it('should support component injection', async () => {
     const event = 'EVENT' as any as Event;
     const url = new URL('http://localhost/path?a=b&c=d');
-    fixture.host.setAttribute(AttributeMarker.ComponentTemplate, String(MyComponent.$templateQRL));
+    fixture.host.setAttribute(AttributeMarker.ComponentTemplate, String(MyComponentTemplate));
 
     const fn = injectEventHandler(
       MyComponent,
@@ -42,8 +42,8 @@ describe('injectEventHandler', () => {
   });
 });
 
+const MyComponentTemplate: QRL = 'myComponentQRL' as any;
 class MyComponent extends Component<any, any> {
-  static $templateQRL: QRL = 'myComponentQRL' as any;
   $newState() {
     return {};
   }

diff --git a/src/core/injector/base_injector.unit.ts b/src/core/injector/base_injector.unit.ts
@@ -82,7 +82,7 @@ describe('BaseInjector', () => {
 
     it('should call injected method', async () => {
       const log: (string | MyComponent)[] = [];
-      fixture.host.setAttribute(AttributeMarker.ComponentTemplate, MyComponent.$templateQRL as any);
+      fixture.host.setAttribute(AttributeMarker.ComponentTemplate, String(MyComponentTemplate));
       const injectedFn = injectMethod(
         MyComponent,
         provideConst('arg0'), //
@@ -103,7 +103,7 @@ describe('BaseInjector', () => {
 
     it('should call injected method (as default)', async () => {
       const log: (string | MyComponent)[] = [];
-      fixture.host.setAttribute(AttributeMarker.ComponentTemplate, MyComponent.$templateQRL as any);
+      fixture.host.setAttribute(AttributeMarker.ComponentTemplate, String(MyComponentTemplate));
       const injectedFn = injectMethod(
         MyComponent,
         provideConst('arg0'), //
@@ -126,10 +126,7 @@ describe('BaseInjector', () => {
 
     describe('error', () => {
       it('should include declare context when throwing error', async () => {
-        fixture.host.setAttribute(
-          AttributeMarker.ComponentTemplate,
-          MyComponent.$templateQRL as any
-        );
+        fixture.host.setAttribute(AttributeMarker.ComponentTemplate, String(MyComponentTemplate));
         const injectedFn = injectMethod(
           MyComponent,
           () => Promise.reject('ProviderRejection'),
@@ -212,8 +209,8 @@ function provideConst<T>(value: T): Provider<T> {
 
 class MyClass {}
 
+const MyComponentTemplate = 'test:/injector/base_injector.unit#template' as any as QRL;
 class MyComponent extends Component<any, any> {
-  static $templateQRL = 'test:/injector/base_injector.unit#template' as any as QRL;
   $newState() {
     return {};
   }

diff --git a/src/core/injector/element_injector.ts b/src/core/injector/element_injector.ts
@@ -64,11 +64,7 @@ export class ElementInjector extends BaseInjector {
     const elementQRL: QRL | null = injector.element.getAttribute(
       AttributeMarker.ComponentTemplate
     ) as any;
-    const $templateQRL = componentType.$templateQRL;
-    if (!$templateQRL) {
-      throw qError(QError.Component_missingTemplateQRL_component, componentType);
-    }
-    if (elementQRL === $templateQRL) {
+    if (elementQRL) {
       let component: COMP = this.component as COMP;
       if (component) {
         if (component instanceof componentType) {

diff --git a/src/core/injector/element_injector.unit.ts b/src/core/injector/element_injector.unit.ts
@@ -12,7 +12,12 @@ import type { EntityKey } from '../entity/entity_key';
 import { stringifyDebug } from '../error/stringify';
 import { QRL } from '../import/qrl';
 import { Entity, Injector } from '../index';
-import { Greeter, GreeterComponent, GreeterProps } from '../util/test_component_fixture';
+import {
+  Greeter,
+  GreeterComponent,
+  GreeterComponentTemplate,
+  GreeterProps,
+} from '../util/test_component_fixture';
 import { ElementFixture, serializeState } from '@builder.io/qwik/testing';
 import { AttributeMarker } from '../util/markers';
 import { getClosestInjector, getInjector } from './element_injector';
@@ -27,10 +32,7 @@ describe('ElementInjector', () => {
 
   describe('getComponent', () => {
     it('should materialize component and return same instance', async () => {
-      fixture.host.setAttribute(
-        AttributeMarker.ComponentTemplate,
-        GreeterComponent.$templateQRL as any
-      );
+      fixture.host.setAttribute(AttributeMarker.ComponentTemplate, GreeterComponentTemplate as any);
       const component = await hostInjector.getComponent(GreeterComponent);
       expect(component).toBeInstanceOf(GreeterComponent);
       expect(stringifyDebug(component.$host)).toEqual(stringifyDebug(fixture.host));
@@ -41,7 +43,7 @@ describe('ElementInjector', () => {
     it('should walk up the tree and find materialize component', async () => {
       fixture.superParent.setAttribute(
         AttributeMarker.ComponentTemplate,
-        GreeterComponent.$templateQRL as any
+        GreeterComponentTemplate as any
       );
       const component = await hostInjector.getComponent(GreeterComponent);
       expect(component).toBeInstanceOf(GreeterComponent);
@@ -50,7 +52,7 @@ describe('ElementInjector', () => {
     it('should return the same promise instance', () => {
       fixture.superParent.setAttribute(
         AttributeMarker.ComponentTemplate,
-        GreeterComponent.$templateQRL as any
+        GreeterComponentTemplate as any
       );
       const component1 = hostInjector.getComponent(GreeterComponent);
       const component2 = hostInjector.getComponent(GreeterComponent);
@@ -60,7 +62,7 @@ describe('ElementInjector', () => {
       it('should materialize from attribute state', async () => {
         fixture.host.setAttribute(
           AttributeMarker.ComponentTemplate,
-          GreeterComponent.$templateQRL as any
+          GreeterComponentTemplate as any
         );
         fixture.host.setAttribute(
           AttributeMarker.ComponentState,
@@ -73,7 +75,7 @@ describe('ElementInjector', () => {
       it('should materialize from $newState', async () => {
         fixture.host.setAttribute(
           AttributeMarker.ComponentTemplate,
-          GreeterComponent.$templateQRL as any
+          GreeterComponentTemplate as any
         );
         fixture.host.setAttribute('salutation', 'Hello');
         fixture.host.setAttribute('name', 'World');
@@ -85,7 +87,7 @@ describe('ElementInjector', () => {
       it('should save state to attribute state', async () => {
         fixture.host.setAttribute(
           AttributeMarker.ComponentTemplate,
-          GreeterComponent.$templateQRL as any
+          GreeterComponentTemplate as any
         );
         const component = await hostInjector.getComponent(GreeterComponent);
         component.$state = { greeting: 'save me' };
@@ -97,25 +99,18 @@ describe('ElementInjector', () => {
     });
     describe('error', () => {
       it('should throw if component does not match', async () => {
-        fixture.parent.setAttribute(AttributeMarker.ComponentTemplate, 'wrongQRL');
         expect(() => hostInjector.getComponent(GreeterComponent)).toThrow(
           "COMPONENT-ERROR(Q-405): Unable to find 'GreeterComponent' component."
         );
       });
-      it('should throw if two components have same $templateQRLs', async () => {
+      it('should throw if two components are of different types', async () => {
         fixture.superParent.setAttribute(
           AttributeMarker.ComponentTemplate,
-          GreeterComponent.$templateQRL as any
+          GreeterComponentTemplate as any
         );
         await hostInjector.getComponent(GreeterComponent);
         expect(() => hostInjector.getComponent(GreeterShadowComponent)).toThrow(
-          "COMPONENT-ERROR(Q-406): Requesting component 'GreeterShadowComponent' does not match existing component 'GreeterComponent'. Verify that the two components have distinct '$templateQRL's."
-        );
-      });
-      it('should throw if two components is missing $templateQRL', async () => {
-        class MissingQRL {}
-        expect(() => hostInjector.getComponent(MissingQRL as any)).toThrow(
-          "COMPONENT-ERROR(Q-407): Expecting Component 'MissingQRL' to have static '$templateQRL' property, but none was found."
+          "COMPONENT-ERROR(Q-406): Requesting component type 'GreeterShadowComponent' does not match existing component instance 'GreeterComponent'."
         );
       });
     });
@@ -223,7 +218,7 @@ describe('ElementInjector', () => {
 });
 
 class GreeterShadowComponent extends Component<GreeterProps, Greeter> {
-  static $templateQRL: QRL = GreeterComponent.$templateQRL;
+  static $templateQRL: QRL = GreeterComponentTemplate;
 }
 
 interface RegardsProps {

diff --git a/src/core/injector/inject.unit.ts b/src/core/injector/inject.unit.ts
@@ -7,7 +7,6 @@
  */
 
 import { injectEventHandler } from '../event/inject_event_handler';
-import type { QRL } from '../import/qrl';
 import { createGlobal, ElementFixture } from '@builder.io/qwik/testing';
 import { AttributeMarker } from '../util/markers';
 import { getInjector } from './element_injector';
@@ -62,7 +61,6 @@ describe('injectEventHandler', () => {
 
   it('should inject this', async () => {
     class MyComp {
-      static $templateQRL = './comp' as any as QRL;
       $state = undefined;
       myComp: boolean = true;
       $newState() {}