Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

no warning message for component reusage in development enviroment #8798

Closed
mastir opened this issue Jan 15, 2017 · 4 comments
Closed

no warning message for component reusage in development enviroment #8798

mastir opened this issue Jan 15, 2017 · 4 comments
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@mastir
Copy link

mastir commented Jan 15, 2017

When i use full version of react everything works fine, no exceptions/warnings. With minified version i've got "Uncaught Error: Minified React error #119; visit http://facebook.github.io/react/docs/error-decoder.html?invariant=119 for the full message or use the non-minified dev environment for full errors and additional helpful warnings." thrown from ReactDom.render on some components.

tested in chrome,firefox. React v15.4.2 from
"https://unpkg.com/react@15/dist/react.js"
"https://unpkg.com/react-dom@15/dist/react-dom.js"

I've found real problem: "component that was not created inside a component's render method". Just like this:

WrapperComponent{
	constructor(props){
	...
	this.props.subcompoent = React.createElement(...)
	}
	render(){
	...
	{this.props.subcomponent}
	...
	}
}

Few instances of this component did render, some subcomponents do work, others - don't. In most cases first subcomponent of type do render fine, others fail. I think there must be some kind of warning for such cases or more info in docs.

@gaearon
Copy link
Collaborator

gaearon commented Jan 15, 2017

Please provide a project reproducing the problem. It is not obvious from your description what is going wrong, but development and production mode should behave identically (except that errors are more verbose in development mode).

For your particular error, it looks like you might be bundling two copies of React on the same page. In fact this is exactly what the error says:

You might be adding a ref to a component that was not created inside a component's render method, or you have multiple copies of React loaded (details: https://fb.me/react-refs-must-have-owner).

Have you checked the suggested documentation page and verified it’s not the case?

@gaearon gaearon added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Jan 15, 2017
@gaearon
Copy link
Collaborator

gaearon commented Jan 15, 2017

Judging by your code snippet, it might be a problem if your this.props.subcompoent has a ref in some cases, and doesn't have it in other cases. But your code doesn't provide enough details to say for sure. However, it still should fail identically in development and production.

By the way, why are you creating this.props.subcompoent separately? It's not really how React is supposed to be used. You don't need to "create" components, you should just include those elements in the render method output. I hope this helps!

@mastir
Copy link
Author

mastir commented Jan 16, 2017

thanks for reply, yes this components created out of render method. I'm using custom loader for roots, just refactored it in search of this issue source and got nothig, works like a charm. The problem is in the ref added by React.cloneElement in constructor. That`s so stupid, its not used.

loader create roots like this:

Url: <UrlInput react-root  key="l#0"/>
Image: <Wrapper react-root type="Image" target_props={...}   key="l#1"/>
Widgets: <Collection react-root   key="l#2">
	<Wrapper  key="l#3" target=<TinyMCEWidget /> />
	<Wrapper  key="l#4" target=<TinyMCEWidget /> />
	<Wrapper  key="l#5" target=<TinyMCEWidget /> />
	<Wrapper  key="l#6" target=<TinyMCEWidget /> />
</Collection>

with minified version first TinyMCE is loaded, then throw exceptions. With dev version everything work fine.

 export class Wrapper extends RComponent<WProps, WState>{

           //executed by loader when collecting props for RElement.
	static parseProps(props:any){
		if(typeof(props.target)=='object'){ //direct
		} else if(typeof(props.component)=='string'){ //legacy
			let type = Loader.Component[props.component]; //get class from string name
			let tprops = (typeof(props.target_props)=='object')?props.target_props:{};
			props.target = React.createElement(type, tprops);
		}
		return props;
	}

	constructor(props){
		super(props);

		this.state.target = React.cloneElement(this.props.target, {
			setUpdated: typeof(this.props.setUpdated)=='function'? this.props.setUpdated : this.setUpdated,
			name: this.props.class_id+':'+this.state.id,
			//ref: 'target' //thats the problem! simple comment it and everything works
		});
	}

	render(){
		return <div className="sci-enity" ref="root">
			{this.state.target}
			{this.getInput()}
			<div className="sci-controll">{this.getControlls()}</div>
		</div>
	}
}

@mastir mastir closed this as completed Jan 16, 2017
@mastir mastir reopened this Jan 16, 2017
@gaearon
Copy link
Collaborator

gaearon commented Jan 16, 2017

First of all, your use of React is highly unidiomatic. It is not clear at all why you create an element in constructor, or why you put it in the state. This is unnecessary and you should generally just create it in the render method instead.

As explained by the error message you have seen, string refs only work when the element is created inside the render method:

You might be adding a ref to a component that was not created inside a component's render method

That is indeed why this doesn't work. This limitation is exactly why we don't recommend string refs anymore, and suggest using callback refs instead. They don't have this problem and are described on this page: https://facebook.github.io/react/docs/refs-and-the-dom.html

At some point string refs will be deprecated so I suggest switching to callback refs.

I hope this helps! I don't see anything actionable for us here. I would recommend refactoring your code to remove the constructor altogether and create the element in render function. If you don't want to do it, you have to replace the string ref with a callback ref.

@gaearon gaearon closed this as completed Jan 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Development

No branches or pull requests

2 participants