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

Adding displayName to the Spec #8

Merged
merged 5 commits into from Jan 29, 2015
Merged

Conversation

ethul
Copy link
Contributor

@ethul ethul commented Jan 28, 2015

Allows the user to specify a display name for the component. This is set automatically by the JSX compiler when using JSX. When the display name is not set, all components are displayed as <Unknown>.

I realize this is adding an additional property to the Spec record. However, I admit that being able to name the components was quite helpful during app development and debugging. If your intention is to keep the Spec small, I could go either way on this. Or perhaps there is a better or alternative approach? I am open to ideas!

Allows the user to specify a display name for the component. This is set
automatically by the JSX compiler when using JSX. When the display name
is not set, all componenents are displayed as <Unknown>.

http://facebook.github.io/react/docs/component-specs.html#displayname
@paf31
Copy link
Owner

paf31 commented Jan 28, 2015

Looks good, but how do you actually specify the display name?

@paf31
Copy link
Owner

paf31 commented Jan 28, 2015

Also, could you please exclude example/index.js?

@ethul
Copy link
Contributor Author

ethul commented Jan 28, 2015

Ah, will do on the exclude for example/index.js.

I was thinking to manually create a spec to specify the display name. But I can add a function displayName similar to componentWillMount. Would this be good?

@paf31
Copy link
Owner

paf31 commented Jan 28, 2015

I was thinking that something like componentWillMount would be nice. Otherwise, the user will have to use SpecRecord directly, right? Really, I'd like to move that type into an internal module.

@ethul
Copy link
Contributor Author

ethul commented Jan 29, 2015

Right. I've added the displayName function.

paf31 added a commit that referenced this pull request Jan 29, 2015
@paf31 paf31 merged commit 7551de9 into paf31:master Jan 29, 2015
@paf31
Copy link
Owner

paf31 commented Jan 29, 2015

👍 thanks!

@ethul
Copy link
Contributor Author

ethul commented Jan 29, 2015

Thank you!

On Wednesday, January 28, 2015, Phil Freeman notifications@github.com
wrote:

[image: 👍] thanks!


Reply to this email directly or view it on GitHub
#8 (comment)
.

@ethul ethul deleted the topic/spec-displayname branch January 29, 2015 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants