supermedium / aframe-react

:atom: Build virtual reality experiences with A-Frame and React.
https://ngokevin.github.io/aframe-react-boilerplate/
MIT License
1.42k stars 151 forks source link

`<Entity camera/>` turns into `<Entity camera="true"/>` #21

Closed ngokevin closed 8 years ago

ngokevin commented 8 years ago

We have to do <Entity camera=""/>. Need to tell aframe-react to not treat those as props or something.

shea-hawkins-zz commented 8 years ago

If a prop exists on props but is null, should we treat it as if it set the component equal to ""?

meta-meta commented 8 years ago

@shea-hawkins when you pass in a prop without a value, React sets the value to true. You don't mean you want to do <Entity camera={null} /> and have aframe-react change it to camera="" right?

shea-hawkins-zz commented 8 years ago

Ah, sorry, I was under the assumption that an empty prop would be set to undefined.

In that case, would we do something like typeof boolean to test if the should be converted to ? This would conflict with future string conversion of component={true} -> component="true".

Do you know a way to tell if it's set to true from React's autoconversion? On Feb 29, 2016 22:23, "Paul M. Christian" notifications@github.com wrote:

@shea-hawkins https://github.com/shea-hawkins when you pass in a prop without a value, React sets the value to true. You don't mean you want to do and have aframe-react change it to camera="" right?

— Reply to this email directly or view it on GitHub https://github.com/ngokevin/aframe-react/issues/21#issuecomment-190551829 .

meta-meta commented 8 years ago

From within the React component, no I don't know of a way to determine if the attribute was explicitly set to true or not. What I think we need is some knowledge on the aframe-react level of the registered aframe components. That way we know that if we have this.props.camera == true and this.props.foo == true we can look and see, oh camera is a registered component, let's convert this to "" but leave foo alone because it's something that aframe doesn't care about.

On a related note, I used to be able to carelessly pass all kinds of props to aframe components and aframe would only look at the ones relevant to the aframe component at hand. Now that I'm on latest dev, that changed and aframe will throw errors when you pass it stuff that doesn't belong. Either I have to rethink my approach to composing aframe-react components or do some introspection on the aframe component schemata to whitelist attributes before shoving them in. I'll give you a more concrete example later.

ngokevin commented 8 years ago

Should those schema errors be warnings?

meta-meta commented 8 years ago

Perhaps. Having extra stuff in the data that the component doesn't care about seems pretty innocuous.

From a purity standpoint, it would be ideal to filter out garbage props before they get into the DOM but I found it more important to get my scene mocked up first.

shea-hawkins-zz commented 8 years ago

I've put in a PR for the original issue, which is fairly important when it comes to making cameras.

I did not see a method for finding the registered aframe components in the docs and I am having a bit of difficulty seeing the use case of checking against them. Because aframe components are the element equivalent, they will never pass props down to children and there don't seem to be any consequences of ""-ing the garbage props that may be present.

meta-meta commented 8 years ago

It's not documented but if you look at aframe dev branch (or in node_modules/aframe) under core/component.js registered components get added to exports.components which ends up as an object on aframe.

We can do import {components} from 'aframe' in react-aframe and see what we got.

On Tue, Mar 1, 2016, 20:54 shea-hawkins notifications@github.com wrote:

I've put in a PR for the original issue, which is fairly important when it comes to making cameras.

I did not see a method for finding the registered aframe components in the docs and I am having a bit of difficulty seeing the use case of checking against them. Because aframe components are the element equivalent, they will never pass props down to children and there don't seem to be any consequences of ""-ing the garbage props that may be present.

— Reply to this email directly or view it on GitHub https://github.com/ngokevin/aframe-react/issues/21#issuecomment-191028972 .

ngokevin commented 8 years ago

yeh window.AFRAME.components or require('aframe').components

ngokevin commented 8 years ago

We changed the schema errors to be warnings

meta-meta commented 8 years ago

Oh awesome. I still want to sanitize but glad I don't have to right away

On Thu, Mar 3, 2016, 01:33 Kevin Ngo notifications@github.com wrote:

We changed the schema errors to be warnings

— Reply to this email directly or view it on GitHub https://github.com/ngokevin/aframe-react/issues/21#issuecomment-191630574 .

ngokevin commented 8 years ago

fixed!