microsoft / angular-react

Use React components inside Angular
https://microsoft.github.io/angular-react/
MIT License
543 stars 73 forks source link

Help Request: React component is checking for a prop when it is created, but the prop doesn't appear to be supplied until later #124

Open RyanHow opened 5 years ago

RyanHow commented 5 years ago

Hi!

I'm trying to create a wrapper around a React component to use within Angular. I haven't done much with React before, so that side of things is quite new to me.

I'm getting an error that a prop is required, but I am supplying that prop.

I had a look and the component is expecting a prop to be set when it is created. However it appears that in Angular-React, the property is supplied later.

The component still works, but it logs a whole lot of errors before it starts working.

The error is like this:

ERROR TypeError: MyReactComponent requires a fetch function.
    at new MyReactComponent (MyReactComponent.js:114)

The code that is throwing the error is this:

if (typeof props.fetch !== 'function') {
  throw new TypeError('MyReactComponent requires a fetch function.');
}

In my template I have (This is slightly simplified, there are multiple related components in there)

<div #reactNode className="myreactcomponent-container">
    <MyReactComponent [fetch]="fetch"></MyReactComponent>
</div>

And fetch is a function in the class (and it is being called)

So I am just wondering if I am doing anything incorrectly, or is there a way I can supply the fetch prop earlier on to avoid the error?

Thanks!

Ryan

aarongreenwald commented 5 years ago

I tried to reproduce but it didn't reproduce for me. What I did: I modified the ReactDot test component to have a required prop and for good measure added a special error in the constructor if the prop was not passed. Then I added the prop in the Angular template. I didn't get any errors. Here's my code:

// tslint:disable:no-input-rename
// tslint:disable:no-output-rename
import {
  Component,
  ChangeDetectionStrategy,
  Input,
  Output,
  EventEmitter,
  ElementRef,
  ViewChild,
  ChangeDetectorRef,
  Renderer2,
  NgZone,
} from '@angular/core';
import * as React from 'react';
import * as PropTypes from 'prop-types';
import { ReactWrapperComponent } from '@angular-react/core';

@Component({
  selector: 'app-react-dot',
  template: `
    <ReactDot
      #reactNode
      [text]="text"
      [foo]="'bar'"
      (onMouseEnter)="onMouseEnter($event)"
      (onMouseLeave)="onMouseLeave($event)"
      [styles]="{
        width: size,
        lineHeight: size,
        height: size,
        left: x,
        top: y,
        color: color,
        backgroundColor: backgroundColor,
        fontSize: size
      }"
    >
      <react-content> <ng-content></ng-content> </react-content>
    </ReactDot>
  `,
  changeDetection: ChangeDetectionStrategy.OnPush,
  styles: ['react-renderer'],
})
export class ReactDotComponent extends ReactWrapperComponent<ReactDotProps> {
  @ViewChild('reactNode') protected reactNodeRef: ElementRef;

  @Input() x: string;
  @Input() y: string;
  @Input() size: string;
  @Input() text: string;
  @Input() color: string;
  @Input() backgroundColor: string;
  @Input() textOverride: string;

  @Output('onMouseEnter') readonly mouseEnter = new EventEmitter<MouseEvent>();
  @Output('onMouseLeave') readonly mouseLeave = new EventEmitter<MouseEvent>();

  onMouseEnter = (ev: MouseEvent) => this.mouseEnter.emit(ev);
  onMouseLeave = (ev: MouseEvent) => this.mouseLeave.emit(ev);

  get computedText() {
    return this.textOverride && this.text ? this.textOverride : this.text;
  }

  constructor(elementRef: ElementRef, changeDetectorRef: ChangeDetectorRef, renderer: Renderer2, ngZone: NgZone) {
    super(elementRef, changeDetectorRef, renderer, { ngZone, setHostDisplay: true });
  }
}

interface ReactDotProps {
  onMouseEnter?: (ev: MouseEvent) => void;
  onMouseLeave?: (ev: MouseEvent) => void;
  text: string;
  foo: string;
  styles?: object;
}

export class ReactDot extends React.Component<ReactDotProps> {
  private static defaultStyle = {
    display: 'block',
    position: 'absolute',
    textAlign: 'center',
    borderRadius: '30%',
    cursor: 'pointer',
  };

  static propTypes  = {
    foo: PropTypes.string.isRequired
  }

  constructor(props) {
    super(props);
    if (!props.foo) {
      throw new TypeError('ReactDot requires a foo prop');
    }
  }

  render() {
    const { text, styles, foo, ...rest } = this.props;

    return React.createElement(
      'div',
      {
        ...rest,
        style: {
          ...ReactDot.defaultStyle,
          ...styles,
        },
      },
      [this.props['text'], ...(this.props.children as any)]
    );
  }
}

The foo prop is the one I added.

aarongreenwald commented 5 years ago

I am able to reproduce the error if I populate the foo prop with a delay, which is not a surprise and is expected. For example:

  template: `
    <ReactDot
      #reactNode
      [text]="text"
      [foo]="bar"
      (onMouseEnter)="onMouseEnter($event)"
      (onMouseLeave)="onMouseLeave($event)"
      [styles]="{
        width: size,
        lineHeight: size,
        height: size,
        left: x,
        top: y,
        color: color,
        backgroundColor: backgroundColor,
        fontSize: size
      }"
    >
      <react-content> <ng-content></ng-content> </react-content>
    </ReactDot>
  `,

and

 constructor(elementRef: ElementRef, changeDetectorRef: ChangeDetectorRef, renderer: Renderer2, ngZone: NgZone) {
    super(elementRef, changeDetectorRef, renderer, { ngZone, setHostDisplay: true });

    setTimeout(() => {
      this.bar = 'bar';
    }, 1000);
  }
RyanHow commented 5 years ago

Thanks for the help! I'm away for a few days, as soon as I'm back I'll try and dig deeper for some more details. I'll put your sample into my setup too and see if it generates any errors.

On 12 Jun 2019, 10:12 pm, at 10:12 pm, Aaron Greenwald notifications@github.com wrote:

I tried to reproduce but it didn't reproduce for me. What I did: I modified the ReactDot test component to have a required prop and for good measure added a special error in the constructor if the prop was not passed. Then I added the prop in the Angular template. I didn't get any errors. Here's my code:

// tslint:disable:no-input-rename
// tslint:disable:no-output-rename
import {
 Component,
 ChangeDetectionStrategy,
 Input,
 Output,
 EventEmitter,
 ElementRef,
 ViewChild,
 ChangeDetectorRef,
 Renderer2,
 NgZone,
} from '@angular/core';
import * as React from 'react';
import * as PropTypes from 'prop-types';
import { ReactWrapperComponent } from '@angular-react/core';

@Component({
 selector: 'app-react-dot',
 template: `
   <ReactDot
     #reactNode
     [text]="text"
     [foo]="'bar'"
     (onMouseEnter)="onMouseEnter($event)"
     (onMouseLeave)="onMouseLeave($event)"
     [styles]="{
       width: size,
       lineHeight: size,
       height: size,
       left: x,
       top: y,
       color: color,
       backgroundColor: backgroundColor,
       fontSize: size
     }"
   >
     <react-content> <ng-content></ng-content> </react-content>
   </ReactDot>
 `,
 changeDetection: ChangeDetectionStrategy.OnPush,
 styles: ['react-renderer'],
})
export class ReactDotComponent extends
ReactWrapperComponent<ReactDotProps> {
 @ViewChild('reactNode') protected reactNodeRef: ElementRef;

 @Input() x: string;
 @Input() y: string;
 @Input() size: string;
 @Input() text: string;
 @Input() color: string;
 @Input() backgroundColor: string;
 @Input() textOverride: string;

@Output('onMouseEnter') readonly mouseEnter = new
EventEmitter<MouseEvent>();
@Output('onMouseLeave') readonly mouseLeave = new
EventEmitter<MouseEvent>();

 onMouseEnter = (ev: MouseEvent) => this.mouseEnter.emit(ev);
 onMouseLeave = (ev: MouseEvent) => this.mouseLeave.emit(ev);

 get computedText() {
return this.textOverride && this.text ? this.textOverride : this.text;
 }

constructor(elementRef: ElementRef, changeDetectorRef:
ChangeDetectorRef, renderer: Renderer2, ngZone: NgZone) {
super(elementRef, changeDetectorRef, renderer, { ngZone,
setHostDisplay: true });
 }
}

interface ReactDotProps {
 onMouseEnter?: (ev: MouseEvent) => void;
 onMouseLeave?: (ev: MouseEvent) => void;
 text: string;
 foo: string;
 styles?: object;
}

export class ReactDot extends React.Component<ReactDotProps> {
 private static defaultStyle = {
   display: 'block',
   position: 'absolute',
   textAlign: 'center',
   borderRadius: '30%',
   cursor: 'pointer',
 };

 static propTypes  = {
   foo: PropTypes.string.isRequired
 }

 constructor(props) {
   super(props);
   if (!props.foo) {
     throw new TypeError('ReactDot requires a foo prop');
   }
 }

 render() {
   const { text, styles, foo, ...rest } = this.props;

   return React.createElement(
     'div',
     {
       ...rest,
       style: {
         ...ReactDot.defaultStyle,
         ...styles,
       },
     },
     [this.props['text'], ...(this.props.children as any)]
   );
 }
}

The foo prop is the one I added.

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/microsoft/angular-react/issues/124#issuecomment-501291900

RyanHow commented 5 years ago

Hi,

Sorry for the delay in getting back to you.

I think I have reduced this down to some interactions with another component on the same parent page.

I moved the other components initialisation into ngAfterViewInit instead of ngOnInit, then all the errors went away.

Digging deeper, the other component is manipulating some DOM (and from what I can tell it is throwing and catching some errors, so usually they don't see the light of day, it all happens internally). This is triggering a zone.js callback by the looks, which is turn is calling the react component, which hasn't initialised yet. (I was originally running this other component outside of the Angular zone, however that seemed to mess up the stack trace and not show where the triggering change that was causing the error). This is a 3rd party component that is minified, so it's a bit hard to follow.

To reproduce the issue:

On the page that holds the react component, add another angular component alongisde it.

In that angular component, in the ngOnInit, throw an error while outside the angular zone like this:

    ngOnInit() {
        this.zone.runOutsideAngular(() => {
            throw new Error('My Error');
        });
    }

This isn't quite the same, because in the original case the error never reaches the console if you run it in isolation, it's only when you run it alongside the react component with a required prop.

I hope this makes some sense!

I'm still trying to get a reproducible isolated case sorry.

RyanHow commented 5 years ago

This is the call stack

printWarning (checkPropTypes.js:26)
checkPropTypes (checkPropTypes.js:82)
validatePropTypes (react.development.js:1666)
createElementWithValidation (react.development.js:1755)
_renderRecursive (angular-react-core.js:545)
(anonymous) (angular-react-core.js:526)
_renderRecursive (angular-react-core.js:526)
render (angular-react-core.js:484)
(anonymous) (angular-react-core.js:717)
end (angular-react-core.js:717)
end (animations.js:332)
end (core.js:36442)
detectChanges (core.js:23797)
detectChanges (elements.js:524)
initializeComponent (elements.js:399)
connect (elements.js:326)
connectedCallback (elements.js:609)
invoke (zone-evergreen.js:359)
onInvoke (core.js:30892)
invoke (zone-evergreen.js:358)
runGuarded (zone-evergreen.js:134)
(anonymous) (zone-evergreen.js:118)
h (DlhSoft.ProjectData.GanttChart.HTML.Controls.js:180)
Aa (DlhSoft.ProjectData.GanttChart.HTML.Controls.js:181)
va (DlhSoft.ProjectData.GanttChart.HTML.Controls.js:318)
ha (DlhSoft.ProjectData.GanttChart.HTML.Controls.js:33)
initBasicDemo (gantt.component.ts:829)
(anonymous) (gantt.component.ts:40)
invoke (zone-evergreen.js:359)
run (zone-evergreen.js:124)
runOutsideAngular (core.js:30818)
ngOnInit (gantt.component.ts:38)
checkAndUpdateDirectiveInline (core.js:24489)
checkAndUpdateNodeInline (core.js:35151)
checkAndUpdateNode (core.js:35090)
debugCheckAndUpdateNode (core.js:36112)
debugCheckDirectivesFn (core.js:36055)
(anonymous) (DashboardComponent.html:9)
debugUpdateDirectives (core.js:36043)
checkAndUpdateView (core.js:35055)
callViewAction (core.js:35421)
execComponentViewsAction (core.js:35349)
checkAndUpdateView (core.js:35062)
callViewAction (core.js:35421)
execEmbeddedViewsAction (core.js:35378)
checkAndUpdateView (core.js:35056)
callViewAction (core.js:35421)
execComponentViewsAction (core.js:35349)
checkAndUpdateView (core.js:35062)
callWithDebugContext (core.js:36395)
debugCheckAndUpdateView (core.js:35978)
detectChanges (core.js:23793)
tick (core.js:32067)
(anonymous) (core.js:31915)
invoke (zone-evergreen.js:359)
onInvoke (core.js:30892)
invoke (zone-evergreen.js:358)
run (zone-evergreen.js:124)
run (core.js:30757)
next (core.js:31912)
schedulerFn (core.js:27834)
__tryOrUnsub (Subscriber.js:183)
next (Subscriber.js:122)
_next (Subscriber.js:72)
next (Subscriber.js:49)
next (Subject.js:39)
emit (core.js:27796)
checkStable (core.js:30835)
onHasTask (core.js:30912)
hasTask (zone-evergreen.js:411)
_updateTaskCount (zone-evergreen.js:431)
_updateTaskCount (zone-evergreen.js:264)
runTask (zone-evergreen.js:185)
drainMicroTaskQueue (zone-evergreen.js:559)
Promise.then (async)
scheduleMicroTask (zone-evergreen.js:542)
scheduleTask (zone-evergreen.js:381)
onScheduleTask (zone-evergreen.js:272)
scheduleTask (zone-evergreen.js:372)
scheduleTask (zone-evergreen.js:211)
scheduleMicroTask (zone-evergreen.js:231)
scheduleResolveOrReject (zone-evergreen.js:845)
resolvePromise (zone-evergreen.js:791)
(anonymous) (zone-evergreen.js:707)
webpackJsonpCallback (bootstrap:25)
(anonymous) (default~app-dashboard-dashboard-module~app-projects-projects-module.js:1)

You'll notice the DlhSoft file in the stack trace. This is what is triggering the issue.

RyanHow commented 5 years ago

Ok, I have a much better test case!

Add a component like this next to the wrapped react component.

So the parent component looks like this (pseudocode)

<react-component [requiredProp]="definedProp"></react-component>
<app-component></app-component>

AppComponent looks like this

import { Component, OnInit, ChangeDetectorRef } from '@angular/core';

@Component({
    selector: 'app-component',
    template: ''
})
export class AppComponent implements OnInit {

    constructor(private changeDetectorRef: ChangeDetectorRef) {}

    ngOnInit() {
        this.changeDetectorRef.detectChanges();
    }
}

Running the change detection cycle during the OnInit seems to be what is triggering the error.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.