core changes

Author: Ben Grynhaus

libs/core/package-lock.json

@@ -1,20 +1,28 @@
 {
   "$schema": "../../node_modules/ng-packagr/package.schema.json",
   "name": "@angular-react/core",
-  "version": "0.1.13-alpha6",
+  "version": "0.2.0-6",
   "ngPackage": {
     "lib": {
       "languageLevel": [
         "dom",
         "es2017"
       ],
+      "embedded": [
+        "css-to-style"
+      ],
       "entryFile": "public-api.ts",
       "umdModuleIds": {
         "react": "React",
-        "react-dom": "ReactDOM"
+        "react-dom": "ReactDOM",
+        "css-to-style": "toStyle"
       },
       "comments": "none"
     },
+    "whitelistedNonPeerDependencies": [
+      "tslib",
+      "css-to-style"
+    ],
     "dest": "../../@angular-react/core"
   },
   "description": "Use React components inside Angular",
@@ -39,9 +47,14 @@
     "@angular/core": "^5.2.7",
     "@angular/platform-browser-dynamic": "^5.2.7",
     "@angular/platform-browser": "^5.2.7",
-    "@types/react-dom": "^16.0.4",
-    "@types/react": "^16.3.4",
     "react-dom": "^16.3.1",
     "react": "^16.3.1"
+  },
+  "dependencies": {
+    "css-to-style": "^1.2.0"
+  },
+  "devDependencies": {
+    "@types/react-dom": "^16.0.4",
+    "@types/react": "^16.3.4"
   }
 }

libs/core/package.json

@@ -1,17 +1,25 @@
-import { AfterViewInit, ComponentFactoryResolver, ComponentRef, ElementRef, Injector, TemplateRef, Type } from "@angular/core";
+import { AfterViewInit, ComponentFactoryResolver, ComponentRef, ElementRef, Injector, TemplateRef, Type, ChangeDetectorRef, OnChanges, SimpleChanges, HostBinding, Input } from "@angular/core";
 import { isReactNode } from "../renderer/react-node";
 import { renderComponent, renderFunc, renderTemplate } from "../renderer/renderprop-helpers";
 import { unreachable } from "../utils/types/unreachable";
+import toStyle from 'css-to-style';
 
-const blacklistedAttributesAsProps = [
-  'class',
-  'style'
+type PropMapper = (value: any) => [string, any];
+
+type AttributeNameAlternative = [string, string | undefined];
+
+const forbiddenAttributesAsProps: ReadonlyArray<AttributeNameAlternative> = [
+  ['key', null],
+  ['class', 'rClass'],
+  ['style', 'rStyle'],
 ];
 
-const blacklistedAttributeMatchers = [
+const ignoredAttributeMatchers = [
   /^_?ng-?.*/
 ];
 
+const ngClassRegExp = /^ng-/;
+
 export interface RenderComponentOptions<TContext extends object> {
   readonly componentType: Type<TContext>;
   readonly factoryResolver: ComponentFactoryResolver;
@@ -31,16 +39,30 @@ export type JsxRenderFunc<TContext> = (context: TContext) => JSX.Element;
  * Simplifies some of the handling around passing down props and setting CSS on the host component.
  */
 // NOTE: TProps is not used at the moment, but a preparation for a potential future change.
-export abstract class ReactWrapperComponent<TProps extends {}> implements AfterViewInit {
+export abstract class ReactWrapperComponent<TProps extends {}> implements AfterViewInit, OnChanges {
 
   protected abstract reactNodeRef: ElementRef;
 
+  @Input() set rClass(value: string) {
+    if (isReactNode(this.reactNodeRef.nativeElement)) {
+      this.reactNodeRef.nativeElement.setProperty('className', value);
+      this.changeDetectorRef.detectChanges();
+    }
+  }
+
+  @Input() set rStyle(value: string) {
+    if (isReactNode(this.reactNodeRef.nativeElement)) {
+      this.reactNodeRef.nativeElement.setProperty('style', toStyle(value));
+      this.changeDetectorRef.detectChanges();
+    }
+  }
+
   /**
    * Creates an instance of ReactWrapperComponent.
    * @param elementRef The host element.
    * @param setHostDisplay Whether the host's `display` should be set to the root child node's `display`. defaults to `false`
    */
-  constructor(public readonly elementRef: ElementRef, private readonly setHostDisplay: boolean = false) { }
+  constructor(public readonly elementRef: ElementRef, private readonly changeDetectorRef: ChangeDetectorRef, private readonly setHostDisplay: boolean = false) { }
 
   ngAfterViewInit() {
     this._passAttributesAsProps();
@@ -50,6 +72,18 @@ export abstract class ReactWrapperComponent<TProps extends {}> implements AfterV
     }
   }
 
+  ngOnChanges(changes: SimpleChanges) {
+    this._passAttributesAsProps();
+  }
+
+  protected detectChanges() {
+    if (isReactNode(this.reactNodeRef.nativeElement)) {
+      this.reactNodeRef.nativeElement.setRenderPending();
+    }
+
+    this.changeDetectorRef.markForCheck();
+  }
+
   /**
    * Create an JSX renderer for an `@Input` property.
    * @param input The input property
@@ -104,15 +138,19 @@ export abstract class ReactWrapperComponent<TProps extends {}> implements AfterV
       (this.elementRef.nativeElement as HTMLElement).attributes
     );
 
-    const whitelistedHostAttributes = hostAttributes.filter(attr => !this._isBlacklistedAttribute(attr));
-    if (!whitelistedHostAttributes || whitelistedHostAttributes.length === 0) {
-      return;
-    }
-
     if (!this.reactNodeRef || !isReactNode(this.reactNodeRef.nativeElement)) {
       throw new Error('reactNodeRef must hold a reference to a ReactNode');
     }
 
+    // Ensure there are no blacklisted props. Suggest alternative as error if there is any
+    hostAttributes.forEach(attr => {
+      const [forbidden, alternativeAttrName] = this._isForbiddenAttribute(attr);
+      if (forbidden) {
+        throw new Error(`[${(this.elementRef.nativeElement as HTMLElement).tagName.toLowerCase()}] React wrapper components cannot have the '${attr.name}' attribute set. Use the following alternative: ${alternativeAttrName || ''}`);
+      }
+    });
+
+    const whitelistedHostAttributes = hostAttributes.filter(attr => !this._isIgnoredAttribute(attr));
     const props = whitelistedHostAttributes.reduce((acc, attr) => ({
       ...acc,
       [attr.name]: attr.value,
@@ -124,7 +162,7 @@ export abstract class ReactWrapperComponent<TProps extends {}> implements AfterV
   private _setHostDisplay() {
     const nativeElement: HTMLElement = this.elementRef.nativeElement;
 
-    // setTimeout since we want to wait until child elements are rendered
+    // We want to wait until child elements are rendered
     setTimeout(() => {
       if (nativeElement.firstElementChild) {
         const rootChildDisplay = getComputedStyle(nativeElement.firstElementChild).display;
@@ -133,11 +171,23 @@ export abstract class ReactWrapperComponent<TProps extends {}> implements AfterV
     });
   }
 
-  private _isBlacklistedAttribute(attr: Attr) {
-    if (blacklistedAttributesAsProps.includes(attr.name)) {
-      return true;
+  private _isIgnoredAttribute(attr: Attr) {
+    return ignoredAttributeMatchers.some(regExp => regExp.test(attr.name));
+  }
+
+  private _isForbiddenAttribute(attr: Attr): [boolean, string | undefined] {
+    const { name, value } = attr;
+
+    if (name === 'key') return [true, undefined];
+    if (name === 'class' && value.split(' ').some(className => !ngClassRegExp.test(className))) return [true, 'rClass'];
+    if (name === 'style') {
+      const style = toStyle(value);
+      // Only allowing style if it's something that changes the display - setting anything else should be done on the child component directly (via the `styles` attribute in fabric for example)
+      if (Object.entries(style).filter(([key, value]) => value || key !== 'display').length > 0) {
+        return [true, 'rStyle'];
+      }
     }
 
-    return blacklistedAttributeMatchers.some(regExp => regExp.test(attr.name));
+    return [false, undefined];
   }
 }

libs/core/src/components/wrapper-component.ts

@@ -12,7 +12,12 @@ export function registerElement(
   resolver: ComponentResolver
 ): void {
   if (elementMap.has(elementName)) {
-    throw new Error(`Element for ${elementName} already registered.`);
+     // Ignore multiple register attempts for the same component.
+     // Angular doesn't allow sharing whole NgModule instances (in this case, an @NgModule for React-wrapped components) with lazy-loaded @NgModules (in the app),
+     // To keep the API simple, allow multiple calls to `registerElement`.
+     // Disadvantage is that you can't replace (React) component implementations at runtime. This sounds far-fetched, but solvable with a `static forRoot()` pattern for every
+     // React-wrapper components' @NgModule, ensuring that `registerElement` is only called once.
+    return;
   } else {
     const entry = { resolver: resolver };
     elementMap.set(elementName, entry);

libs/core/src/renderer/registry.ts

@@ -56,7 +56,7 @@ export class AngularReactRendererFactory extends ɵDomRendererFactory2 {
 }
 
 class ReactRenderer implements Renderer2 {
-  data: { [key: string]: any } = Object.create(null);
+  readonly data: { [key: string]: any } = Object.create(null);
 
   constructor(private rootRenderer: AngularReactRendererFactory) { }
 

libs/core/src/renderer/renderer.ts

@@ -0,0 +1,7 @@
+
+declare module 'css-to-style' {
+
+  function toStyle(cssText: string) : CSSStyleDeclaration;
+
+  export default toStyle;
+}