React-native-gifted-chat: Code style guide

[dgdavid]

@kfiroo what about use a common code style guide?

Currently the project has some eslint dependencies - which should be updated - and the ./node_modules/eslint/bin/eslint ./src command output

✖ 28 problems (28 errors, 0 warnings)

Now that the project activity was relaunched could be time of starting to use a common code style guide and lint the code before merge it. So, Why not to use a CI tool like Travis to check this on every PR?

If you want, we can discuss which to use (I prefer the Airbnb guides) and configure it. I've been trying some stuff related to this in my fork, but nothing finished yet.

BTW, talking about CI, maybe is also time to start adding tests. New Year's resolutions? ;)

[cooperka]

+1 to Airbnb style, I've been using esling-config-airbnb + eslint-config-react + eslint-config-react-native everywhere and it's great.

As far as tests, I'm a fan of Jest snapshots. If you snapshot-test each component with various combinations of props (this logic can easily be abstracted away and automated), then you don't actually need to write a single "real" test and you'll have 100% render logic covered, plus you can see what each rendered component looks like in every state without having to boot up the app yourself (the snapshots are saved as plain text, and can be re-generated after each code change).

In my experience snapshots are basically all that's necessary; interaction between components rarely changes, and when it does, it's much easier to test quickly by hand for sanity, and will always be quickly reported when something does go wrong.

Love to hear more thoughts!

[kfiroo]

@dgdavid eslint with CI to check the PRs would be an amazing addition to this project! and when we add some tests it will be even better!
@cooperka can we use your eslint config? let's do it!

@cooperka I never worked with Jest or it's Jest snapshot, any change you can put together a simple demo?

[dgdavid]

@cooperka I read about Jest snapshots but I have not used it yet. However it seems a good idea to me.

Thanks for create and share your eslint config. I'll to take a look :)

[cooperka]

@kfiroo you can absolutely use it! I'll update my README tonight to make it more clear, but you basically just install it and add "extends": "cooperka/react-native" to your .eslintrc. I can submit a PR for this :)

For Jest snapshots, I have a project that currently uses them here. It's not very clean right now tbh, but in essence you just use

const tree = renderer.create(<ImmutableListView someProp="foo" />);
expect(tree.toJSON()).toMatchSnapshot();

and get an output like

exports[`ImmutableListView renders with someProp`] = `
<ScrollView
  someProp="foo">
  <Text>
    Hello, foo!
  </Text>
</ScrollView>
`;

Then if someone ever changes anything -- e.g. props, styles, children, anything at all -- the snapshot will also be updated and committed along with the PR. It's a good way to verify that your change does exactly what you expect and doesn't break any other components.

You can try it yourself by running npm test on that repo I linked.

[dgdavid]

@cooperka,

At sight view, I think that https://github.com/FaridSafi/react-native-gifted-chat/pull/668 solve this issue, isn't it? If so, we can close it.

---

[Edit] just a curiosity: I just realized that I am writing here an exact year after open the proposal.

[cooperka]

Hurrah for ESLint! I think we can close this.

Or maybe we should wait for unit tests, too? I still love Jest snapshots as described above.

[dgdavid]

Of course, hurrah for ESLint 🎉 !

Regarding close or not the issue, I leave it in your hands 😉. As you could notice, I am totally disconnected from the project in participation terms 😢. You know, real life and its stuff. So, despite I try to keep tracking its development, I do not have a complete project's big picture right now to give an opinion about keep it open waiting for unit tests.

Perhaps a new specific issue for them?

Regards!

[cooperka]

Test discussion forked to https://github.com/FaridSafi/react-native-gifted-chat/issues/671, closing!