Chalarangelo/30-seconds-of-react

[CHORE] Snippet Template

Chalarangelo opened this issue ยท 21 comments

We don't have a template for snippets in this repository yet. We should figure this out and create a template as soon as possible.

How about

// Expected props = {}

constructor(props) {} // optional

function1 = () => 

function2 = () => 

render() {} // optional

What about examples?

What about examples?

The whole snippet could be in the form of an example I guess. Like how they show react components in docs of react-mdl, material-ui etc

It depends on the snippet. We should only use a class if it's going to be a stateful component. Often times you can get away with none stateful components.

Since React is usually built(although now with import in browsers it might be a thing of the past), we can expect the language features of ES6 to be present. As such I suggest we use destructuring when possible after the super call, and if defaults are needed we then check those soon after(this is useful in certain situations like models, where we need a state that only has a show or hide), and destructuring allows for default values.

As for order of operations, it's usually good to organize it in a top down manner of importance and then complexity:

  1. Things that need to happen before anything custom
    a. React/Library Things
    b. Defaults and Destructuring
  2. Assignment to instance
    a. State
    b. Modifications
    c. Properties
  3. Creation or binding of functions
    a. Binds
    b. Arrow Function Declarations
class ModalExample extends Component{
  constructor (props) {
    super(props);

    let {
      show = false,
      children = <div />,
      className = 'modal'
    } = props;

    this.state = {
      show
    };

    this.props.children = this.props.children || children;
    this.props.className = this.props.className || className;

    this.render = this.render.bind(this);

    this.toggle = () => this.setState(oldState => ({ show: !oldState.show }));
  }

  render(){
    return this.state.show
      ? <div onClick={this.toggle} className={this.props.className}>{ this.props.children }</div>
      : null
  }
}

While this is a simple example of a stateful Class component, there will be functional components that will fit better in other places(like a photo gallery), those will still follow the same order, just they won't be a class and often won't implement most of the things, but by following the same order...

function functionalSnippet (props) {
    let {
      className = 'snippet',
      children = <div />
    } = props;

    props.className = props.className || className;
    props.children = props.children || children;

    return <div className={props.className} >{props.children}</div>
}

I can easily find my default values and then assignments of those default values. Just as long as the order is decided on and maintained then our snippets will be quick to pick up on. Readability and maintainability take a primary focus here, and that means consistency!

Oh and before you ask, yes... I have adopted this order for pretty much every class or function I do that isn't a simple Lambda.

A side note, I forgot about React.Component.defaultProps{} capabilities... That would however change the order and readability of the snippets since they get defined AFTER the class which would mean we'd on them on a separate lines before the class, and then assigned to the property on the class.

This does not work for functional components however, so it might still be better to use the more verbose version proposed above for consistency or deciding on if we should honor the difference between functional components and class components(which would require definition)

The proposed version shouldn't be an issue though since props are unique to the instance unless you're passing them down... which... why would you?

I like what @skatcat31 suggests. My remarks on the whole issue are as follows:

  • We will definitely be able to go with ES6. We did it in 30code, we can do it here and it's actually a bit safer to assume that the code will be compiled when using React, so we will have fewer issues.
  • Snippets will most likely be examples, not plug-n-play as 30code snippets are, so we might as well do our best to make them easy to tweak. I don't expect we will need a separate example (usage) section like we have done in 30code, but it remains to be seen.
  • We can use functional components whenever possible, classes otherwise. That will help introduce people to both styles of components.
  • Organization of tasks as @skatcat31 suggested is entirely fine by me, I like it.
  • constructor - everything_else - render is a good flow for the code in my opinion.
  • We should also define a specific order for any lifecycle events that we handle in the components, for consistency.

well either compiled or imported... since React itself can't work without ES6...

As for order of life cycle events we could use one of two thoughts:

for props -> then should

before then -> will then -> did

However with the recent updates in React 16+ we may want to take a closer look at that

@skatcat31 I didn't quite catch that last one, care to elaborate?

SCHKN commented

@skatcat31 Do you think defaultProps are relevant elements to show on snippets? I feel like they are not used enough for them to be considered

@SCHKN @Chalarangelo

In order for the snippet to work once copied, we should include defaultProps. Maybe we shouldn't display it on the website (to save space) but we can definitely have it hidden and copied once the user wants to copy the snippet to the clipboard ๐Ÿ‘

@SCHKN @Chalarangelo

In order for the snippet to work once copied, we should include defaultProps. Maybe we shouldn't display it on the website (to save space) but we can definitely have it hidden and copied once the user wants to copy the snippet to the clipboard

Does it really have to work on copying the exact thing?

SCHKN commented

Should the signature of the component be added as a comment before declaring the component, as a use case?

// <ModalExample myProp="disabled" />

class ModalExample extends Component{
  constructor (props) {
    super(props);
    this.state = {  show : true };
  }

  render(){
    return this.state.show
      ? <div onClick={this.toggle} className={this.props.myProp}>Example</div>
      : null
  }
}
ModalExample.defaultProps = { myProp: 'active' };

It could be added before in the template under some tag (see other 30s templates). Once we build a website we can find a better place for it :)

@fejes713 NEVER hide required or provided information. That is an extremely bad practice. "Why did this work here, but not here?" 'Oh this magic number from a file two directories up' "Why wasn't it documented?" '......'

Again I vote against the Class.defaultProps use as it would be inconsistent, and worse yet is going to be overridden by class properties at some point or just put them in the constructor so we can find all declarations in a single place.

If we do adopt them we should put them declared as an object before, and then assigned after deceleration of the class

const defaultProps = {...};

class Snippet{...}
Snippet.defaultProps = defaultProps;

This way we can keep declarations BEFORE logic.

I would still suggest we do destructuring and assignment instead since if defaultProps ever becomes deprecated we'd want to be ahead of the curve with just using language features(especially if class variable declaration moves forward, no matter how much I hate that suggestion to TC39... If we allowed more than one constructor I'd be instantly on board for it though... thank god we don't though... I'm not even sure ES could support overloaded constructors)

@Chalarangelo didn't catch which part? The order suggestion or the changes in React 16?

@fejes713 NEVER hide required or provided information. That is an extremely bad practice. "Why did this work here, but not here?" 'Oh this magic number from a file two directories up' "Why wasn't it documented?" '......'

@skatcat31 Yeah right. Makes sense. I agree that this isn't the best idea.

Updated proposal

  • There should be an explanatory guide/handy cheatsheet at the top as mentioned in #1. This will help us, at least early on. We can always move away from this idea later down the line.
  • Cards/Snippet Files will contain the following as part of their definition
    • Title: at the top, name of the snippet like in every other project Unique project-wide
    • Description: Short and sweet, what does the code accomplish?
    • Actual code: Read below for formatting
    • Explanation: Explain how the code works, as always
    • Example: Short example of the snippet in action - This might be compiled or namespaced and it will probably be shown as plain code in the website with a link to an external Codepen or repl.it for users to fiddle around with it (let's not integrate all of the snippets right away and bloat the website)
    • Tags: Either in the snippet or in a database file like in 30code, whatever works best
  • Functional components will be preferred over class components, if both would work in a certain scenario.
  • ES6 syntax. Assume people compile their code, we did so in 30code where that assumption was more error-prone.
  • Ordering of things:
  1. React/Library things
    a. React/Library Things
    b. Defaults and Destructuring
  2. Assignment to instance
    a. State
    b. Modifications
    c. Properties
  3. Creation or binding of functions
    a. Binds
    b. Arrow Function Declarations
  • Ordering of code:
  1. constructor
  2. everything_else
  3. render
  • Naming should not deal with functional or class in the name, but we can enforce one of the two tags to be present in every snippet to make the whole process easier.

@skatcat31 @flxwu @fejes713 @atomiks Opinions so we can conclude on a template and start working?

@Chalarangelo I stated function or class should be in the filename, not the snippet name ๐Ÿ˜‰ but again if it's tagged somewhere I don't care too much.

As for tagging I like it a lot better the way Interviews did, with tags maintained in a comment in the file so that text transformers can find it, but it won't display.

Otherwise this looks about right for a format.

lock commented

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for any follow-up tasks.