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

Adds decorators #31

Merged
merged 1 commit into from Aug 6, 2015
Merged

Conversation

goatslacker
Copy link
Contributor

Floating this change with you, if it's cool to add then I'll add some tests and docs.

I know this is a bit futuristic but this makes the API really nice to work with. Check out the Stargazers example.

@@ -5,7 +5,7 @@ module.exports = {

module: {
loaders: [
{ test: /\.js$/, exclude: /node_modules|dist/, loader: "babel-loader" },
{ test: /\.js$/, exclude: /node_modules|dist/, loader: "babel-loader?stage=0" },
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think stage=1 should be enough here (decorators are a Stage 1 feature).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stage 0 is so static class properties work which are required so that the class has a display name.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the notes here. I missed this (subtle) difference. I like being able to do propTypes inside of the class definition, too!

@ericclemmons
Copy link
Owner

@goatslacker Can you shed some more light on how decorators work in this scenario? Like, for the end-user? I've seen David Abromov's (SP?) experiments with it, but don't know much besides that.

I kinda still don't get it:

https://github.com/wycats/javascript-decorators

@goatslacker
Copy link
Contributor Author

The de-sugared version of this would be (it's just a function call):

Resolver.decorate(options)(Component)

Don't mind the name, we can call it anything else.

@ericclemmons
Copy link
Owner

We can call it whatever you want, suga!

/rimshot?

@ericclemmons ericclemmons mentioned this pull request Apr 26, 2015
@ericclemmons
Copy link
Owner

@goatslacker Question for ya. I've been messing with decorators internally and was naturally drawn to:

  • @Capitalized decorators when they wrap classes.
  • @lowerCased decorators for properties & methods.

The examples are non-opinionated as well:

https://github.com/wycats/javascript-decorators#class-declaration

If course, now eslint complains because Capitalized(function() { ... }) should be invoked with new.

What are your thoughts? My initial take on them, where they mirror the naming/capitalization of their subject, or always lowercased because they're technically functions and should be named as such?

@goatslacker
Copy link
Contributor Author

I've generally always kept them camelCase because like you said they're functions, but I don't have any strong opinions on the matter.

@ericclemmons ericclemmons merged commit e8e33bd into ericclemmons:master Aug 6, 2015
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

4 participants