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

'export default component' syntax should be removed from 'rcc' snippet #8

Closed
OshotOkill opened this issue Mar 10, 2016 · 5 comments
Closed

Comments

@OshotOkill
Copy link

A normal React component is not always be the one which should be exported, so rcc snippet should only create a class without export default component syntax and instead replace it with rcced or something else.

By the way it would be better if 'con' snippet creates constructor(props, context) rather than constructor(props).

Thank you for founding this convenient extension, I believe both of these changes are not difficult to implement, right?

@xabikos
Copy link
Owner

xabikos commented Mar 10, 2016

Thanks for the feedback OshotOkill.

Regarding the default action of exporting the component with rcc snippet could you give an example when a component is not exported by default? I have in mind the practice of having only one component per file. The only case I can think of is the "smart" components in a redux application. A different set of snippets particularly for redux are in my plans for the near future.

Now about a snippet for a constructor that except props contains also the context is really easy to add. I didn't do it in advance as context is an advance feature and it's usage is discouraged actually but I am happy to add it. I guess that conc is a good candidate.

@OshotOkill
Copy link
Author

Yeah only a single component per file is one of the best practice in React/Redux while not every one intends to implement it, whether they don't have a distinct structure in mind to export which component at the beginning or they just tend to export a component but nested with others, and with that, we have to scroll down to the end line of the file to delete unnecessary export default sentence. That's the reason why I prefer to move export default to another snippet such as rcced, more choice more better.

As to add context parameter in constructor signature, my thought comes with the default constructor
in react

function ReactComponent(props, context, updater) {
  this.props = props;
  this.context = context;
  this.refs = emptyObject;
  this.updater = updater || ReactNoopUpdateQueue;
}

Though context is discourage however it's still being used widely in many project specially passing css-styles into lower components. Since JS extends instance constructor it is a error prone which would cause losing context property. Even if we don't use context at all but add context may enhance the robustness of components. React-Router, Redux already did that.

@xabikos
Copy link
Owner

xabikos commented Mar 11, 2016

Thanks for the input. I would prefer not to change the behavior of the existing snippet rcc in order not to confuse existing users but instead add a new one that doesn't export the component by default. But in that case I am just thinking maybe this snippet should not have import statements too to avoid not required import lines in multiple components per file. What do you think of a snippet like rcjc from React create just component?

Now about context constructor parameter I created #9

@OshotOkill
Copy link
Author

Yeah rcjc is ok, we need the way to create just a pure component, so the import statement should be removed as well which I forgot to say (same with stateless component).

Thanks and forgive my wayward requirements.

@xabikos
Copy link
Owner

xabikos commented Mar 14, 2016

Implemented in the latest version 0.4.0

@xabikos xabikos closed this as completed Mar 14, 2016
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

No branches or pull requests

2 participants