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

Any ideas about some other helpers that could be included in this repo? #5

Open
darrenscerri opened this issue Nov 8, 2015 · 12 comments

Comments

@darrenscerri
Copy link

No description provided.

@jguyon
Copy link

jguyon commented Nov 8, 2015

I was thinking about a decorator that would transform a stateless function component into a component class. It could be useful when you want to apply a decorator that needs a class as input.

For example:

@someDecorator
class Person extends Component {
  @injectProps
  render({ firstName, lastName, email }) {
    return <p>{ firstName } { lastName } - { email }</p>;
  }
}

...would look like this:

@someDecorator
@makeStateful
const Person = ({ firstName, lastName, email }) => (
  <p>{ firstName } { lastName } - { email }</p>
);

I'm not sure about the name, but I thought makeStateful made it clear that it was changing the nature of the component.
What do you think?

@goncalvesjoao
Copy link
Owner

@jguyon the thing is decorators only work with classes, and "const Person = () => (...);" is just a function.

But you could make a javascript function to help you with that:

function makeStateful(renderFunction) {
  return class StatefulComponent extends React.Component {
    render() { return renderFunction(this.props); }
  };
}

export default makeStateful;
import makeStateful from './makeStateful';

const Person = makeStateful(({ firstName, lastName, email }) => (
  <p>{ firstName } { lastName } - { email }</p>
));

export default Person;

If you don't mind me asking, why do you feel the need to have a stateless component with state?
Can you make an example?

Thanks

@jguyon
Copy link

jguyon commented Nov 10, 2015

@goncalvesjoao I'm sorry, I badly articulated my thoughts and I chose a very bad name for the helper.
Also, nevermind the example I provided in my first comment, it was not the use case I was thinking about initially.

I chose makeStateful as a name because components written as functions are called stateless components in the docs (as opposed to stateful components, which are written as classes). But what I want is not really to make it stateful, but for it to have a backing instance.

Because stateless function components don't have a backing instance, it means you can't access their node via findDOMNode. In my tests, this leads me to write a wrapper component in each test file, which is kind of annoying. A helper like this would eliminate a little boilerplate.
The example I provided initially was just me thinking it could have another use.

@goncalvesjoao
Copy link
Owner

Sorry for the late replay @jguyon.
Oh I see, yeah man I had the same pain when testing stateless functions.
Ended up making a helper function, check this small example:

In on of my apps I have a small CloseButton Component:

import { killEvent } from 'relpers';

const CloseButton = ({ id, closeTab, stickable }) => (
  stickable
    ? <noscript />
    : <span
        className="glyphicon glyphicon-remove"
        onClick={ killEvent(() => closeTab(id)) }
      />
);

export default CloseButton;

Its spec is really simple too, but needs this helper function first:

import TestUtils from 'react-addons-test-utils';
import { findDOMNode } from 'react-dom';

function renderStateless(Component, props) {
  const wrapper = TestUtils.renderIntoDocument(<div><Component { ...props } /></div>);

  return findDOMNode(wrapper).children[0];
}

export default renderStateless;

CloseButtonSpec.js:

import chai from 'chai';
import React from 'react';
import sinon from 'sinon';
import sinonChai from 'sinon-chai';
import TestUtils from 'react-addons-test-utils';
import renderStateless from '../support/renderStateless';

chai.use(sinonChai);

import CloseButton from '../../../src/nav-tabs/components/CloseButton';

describe('CloseButton', () => {
  context('when stickable prop is true', () => {
    it('<noscript /> should be returned', () => {
      const closeButton = renderStateless(CloseButton, { stickable: true });

      // see: https://developer.mozilla.org/en-US/docs/Web/API/Node
      expect(closeButton.nodeName).to.be.equal('NOSCRIPT');
    });
  });

  context('when stickable props is not used or false', () => {
    it('closeTab prop callback should be called with id prop in it', () => {
      const closeTabStub = sinon.spy();
      const closeButton = renderStateless(CloseButton, { id: 1, closeTab: closeTabStub });

      // see: https://facebook.github.io/react/docs/test-utils.html
      TestUtils.Simulate.click(closeButton);

      // see: http://chaijs.com/plugins/sinon-chai
      expect(closeTabStub).to.be.calledWith(1)
    });
  });
});

@goncalvesjoao
Copy link
Owner

The example above solves our need but checkout this tool: https://github.com/jquense/teaspoon

I ended up remaking the CloseButtonSpec with teaspoon and it looks like this:

import chai from 'chai';
import $ from 'teaspoon';
import React from 'react';
import sinon from 'sinon';
import sinonChai from 'sinon-chai';

chai.use(sinonChai);

import CloseButton from '../../../src/nav-tabs/components/CloseButton';

describe('CloseButton', () => {
  context('when stickable prop is true', () => {
    it('<noscript /> should be returned', () => {
      const $closeButton = $(<CloseButton stickable={ true } />).render();

      // see: https://developer.mozilla.org/en-US/docs/Web/API/Node
      expect($closeButton.dom().nodeName).to.be.equal('NOSCRIPT');
    });
  });

  context('when stickable props is not used or false', () => {
    it('closeTab prop callback should be called with id prop in it', () => {
      const closeTabStub = sinon.spy();
      const $closeButton = $(<CloseButton id={ 1 } closeTab={ closeTabStub } />).render();

      // see: https://github.com/jquense/teaspoon
      $closeButton.trigger('click');

      // see: http://chaijs.com/plugins/sinon-chai
      expect(closeTabStub).to.be.calledWith(1)
    });
  });
});

cool tool uh?
It works with both state and stateless components out-of-the-box and due to its "jQuery" nature, there is no need for refs.

@sergiodxa
Copy link
Contributor

How about a decorator to apply mixins to give retrocompatibility with mixins like the one in yahoo/react-intl 1.x. Something like this:

function applyMixin(mixin) {
  return target => {
    const mixed = Object.assign({}, mixin, target.prototype);
    target.prototype = mixed;
    return target;
  };
}

Then you can just do:

import React from 'react';
// import mixin
import {
  IntlMixin,
} from 'react-intl';
// import decorator
import {
  applyMixin,
} from 'relpers';

// apply mixin to ES6 class
@applyMixin(IntlMixin)
class MyComponent extends React.Component {
  render() {
    return (
      <div>
        { this.getIntlMessage('message') }
      </div>
    );
  }
}

export default MyComponent;

I don't know if that is the best way to code that decorator, but it works and if your component has a method with the same name then the component method overwrite the mixin method. Also, I don't tested if this works with things like getChildContext and makes the merge if you had your own getChildContext, like the mixins in createClass() do.

Example working (without React, but the idea is the same): http://codepen.io/sergiodxa/pen/YwNjdJ?

@goncalvesjoao
Copy link
Owner

that's actually a great ideia @sergiodxa

@goncalvesjoao
Copy link
Owner

Check it out @sergiodxa I've made a branch for you: https://github.com/goncalvesjoao/relpers/tree/applyMixin

It contains Specs and documentation for your ideia, but it doesn't include the code for the decorator.
Wanna help? ;)

You just need to fork this repo, checkout the applyMixin branch, run npm install, then npm test to see the specs failing and fill in the blanks of src/applyMixin.js untill all tests are green.

When all specs are green you should be able to see the example at: http://localhost:9000/api_docs/apply_mixin work
To start up the server, run npm start

If you have any doubts or don't feel like it, let me know.

@sergiodxa
Copy link
Contributor

I just create the Pull Request with the decorator ready.

@goncalvesjoao
Copy link
Owner

Thanks for such a thorough PR man ✋

@goncalvesjoao
Copy link
Owner

@sergiodxa sorry, I accepted your PR but forgot to publish it to npm.
Did it just now if you want to try it out ;)

@sergiodxa
Copy link
Contributor

I have another idea, but I think is complicated, I think it will be really helpful a decorator the can detect if the user is logged and render a component, something like @onlyLogged decorator.

I think the best way is that the decorator need to be a React.Component that check the contextType to validate if this.context.isLogged (or some other context key, maybe it can be custom) is true and render the decorated component, else render null, other component, or execute a function (in this function you can redirect the user to other route) or something else.

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

4 participants