While we mostly use kebab case for file names with no jsx extension. The proposed guidelines by AirBnb are very compelling. What are everyone else's thoughts? Below is the except from: https://github.com/airbnb/javascript/tree/master/react#naming

Naming

• Extensions: Use .jsx extension for React components.

• Filename: Use PascalCase for filenames. E.g., ReservationCard.jsx.

• Reference Naming: Use PascalCase for React components and camelCase for their instances. eslint: react/jsx-pascal-case

  // bad
  import reservationCard from './ReservationCard';
  
  // good
  import ReservationCard from './ReservationCard';
  
  // bad
  const ReservationItem = <ReservationCard />;
  
  // good
  const reservationItem = <ReservationCard />;

• Component Naming: Use the filename as the component name. For example, ReservationCard.jsx should have a reference name of ReservationCard. However, for root components of a directory, use index.jsx as the filename and use the directory name as the component name:

  // bad
  import Footer from './Footer/Footer';
  
  // bad
  import Footer from './Footer/index';
  
  // good
  import Footer from './Footer';

• Higher-order Component Naming: Use a composite of the higher-order component's name and the passed-in component's name as the displayName on the generated component. For example, the higher-order component withFoo(), when passed a component Bar should produce a component with a displayName of withFoo(Bar).

  Why? A component's displayName may be used by developer tools or in error messages, and having a value that clearly expresses this relationship helps people understand what is happening.

  // bad
  export default function withFoo(WrappedComponent) {
    return function WithFoo(props) {
      return <WrappedComponent {...props} foo />;
    }
  }
  
  // good
  export default function withFoo(WrappedComponent) {
    function WithFoo(props) {
      return <WrappedComponent {...props} foo />;
    }
  
    const wrappedComponentName = WrappedComponent.displayName
      || WrappedComponent.name
      || 'Component';
  
    WithFoo.displayName = `withFoo(${wrappedComponentName})`;
    return WithFoo;
  }

• Props Naming: Avoid using DOM component prop names for different purposes.

  Why? People expect props like style and className to mean one specific thing. Varying this API for a subset of your app makes the code less readable and less maintainable, and may cause bugs.

  // bad
  <MyComponent style="fancy" />
  
  // bad
  <MyComponent className="fancy" />
  
  // good
  <MyComponent variant="fancy" />

Does this file serve a purpose? I can't seem to find a reference to it anywhere.

https://github.com/ciscospark/web-styleguide/blob/master/packages/node_modules/%40ciscospark/eslint-config-base/rules/legacy.js

When we originally set requireReturn: true, it was there to ensure that we always documented a return type when there was one. A side effect was that it makes you type @returns {undefined} a lot.

Sometime between when we first picked that rule and now, eslint got smarter with requireReturn: false such that it still forces you to document return if the function returns anything.

REALLY not a fan of the "before" setting of operator-linebreak. Highly prefer "after". Thoughts?

// "after" setting
var fullHeight = borderTop +
                 innerHeight +
                 borderBottom;

vs

// "before" setting
var fullHeight = borderTop
               + innerHeight
               + borderBottom;

https://eslint.org/docs/2.0.0/rules/operator-linebreak

https://github.com/ciscospark/web-styleguide/blob/master/packages/node_modules/%40ciscospark/eslint-config-base/rules/style.js#L379

With newlines-between set to always it enforces no new lines with between imports within a group. Being able to add a new line between vendor imports and our own namespaced @ciscospark imports would help with readability

Current rule:

    'import/order': ['error', {
      groups: ['builtin', 'external', 'internal', 'parent', 'sibling', 'index'],
      'newlines-between': 'always'
    }],

Proposed rule:

    'import/order': ['error', {
      groups: ['builtin', 'external', 'internal', 'parent', 'sibling', 'index'],
      'newlines-between': 'always-and-inside-groups'
    }],

🚨 You need to enable Continuous Integration on all branches of this repository. 🚨

To enable Greenkeeper, you need to make sure that a commit status is reported on all branches. This is required by Greenkeeper because it uses your CI build statuses to figure out when to notify you about breaking changes.

Since we didn’t receive a CI status on the greenkeeper/initial branch, it’s possible that you don’t have CI set up yet. We recommend using Travis CI, but Greenkeeper will work with every other CI service as well.

If you have already set up a CI for this repository, you might need to check how it’s configured. Make sure it is set to run on all new branches. If you don’t want it to run on absolutely every branch, you can whitelist branches starting with greenkeeper/.

Once you have installed and configured CI on this repository correctly, you’ll need to re-trigger Greenkeeper’s initial pull request. To do this, please delete the greenkeeper/initial branch in this repository, and then remove and re-add this repository to the Greenkeeper App’s white list on Github. You'll find this list on your repo or organization’s settings page, under Installed GitHub Apps.

Some third-party libraries expose their api with underscore-prefixed properties that are supposed to be used. It would be nice if this produced a warning:

const x = {
  _x: false
}

but this did not:

thirdparty._whyDoesItStartWithAnUnderscoreIfYouWantMeToUseIt()

Should be '@webex/eslint-config-base',

Fixable with the following config

  require-jsdoc:
    - error
    - require:
        FunctionDeclaration: true
        MethodDefinition: true
        ClassDeclaration: true
        ArrowFunctionExpression: false
        FunctionExpression: true

Generally, I'm a fan of setting no-warning-comments: error since they get lost really easily and never acted upon. Historically, we've had no-warning-comments: warn in place since there are already so many in the code base. At a minimum, I'd like to up that to warn and, ideally, error.

https://eslint.org/docs/rules/no-template-curly-in-string

With the switch away from backticks for non-templates, it's easy to create a template string with apostrophes.

Same issue we've seen with our other monorepos. I think I determined that there is a way to fix it, but our build process has historically been too slow to experiment with.