This repo shall serve as the basis for all code quality tools used in STRV, so let's move the ESLint config in. 💪

babel-eslint has been deprecated. This package will no longer receive updates.

https://www.npmjs.com/package/babel-eslint

Hey, I came across to this during installation, so just FYI :]

Current:

# Yarn:
yarn add --dev @strv/stylelint-config

# npm:
npm install --save-dev @strv/stylelint-config

Expected:

# Yarn:
yarn add --dev @strv/stylelint-config-styled-components

# npm:
npm install --save-dev @strv/stylelint-config-styled-components

Hi!

Firstly I would like to thank you for providing migration tutorial!

I would suggest mentioning this issue in that tutorial, specifically that this new package is not compatible with @strv/react-scripts v3.0.0

If I manage to add either @strv/react-native/style or @strv/eslint-config-typescript/style to my .eslintrc, I get this error:

Error: ../../.eslintrc.js » @strv/eslint-config-react-native/style » /Users/matheusalbuquerque/Documents/dev/strv/goulash-club/Repos/goulash-club-frontend/node_modules/@strv/eslint-config-react/style.js » /Users/matheusalbuquerque/Documents/dev/strv/goulash-club/Repos/goulash-club-frontend/node_modules/@strv/eslint-config-base/style.js:
	Configuration for rule "import/no-useless-path-segments" is invalid:
	Value {"noUselessIndex":true} should NOT have additional properties.

I checked the source and it seems to be just like to documentation reccomends – so, for now, I don't have any clue 😞 .

Dependabot can't resolve your JavaScript dependency files.

As a result, Dependabot couldn't update your dependencies.

The error Dependabot encountered was:

Error whilst updating eslint-plugin-mocha in /packages/eslint-config-mocha/package-lock.json:
404 Not Found - GET https://registry.npmjs.org/@strv/eslint-config-base - Not found

If you think the above is an error on Dependabot's side please don't hesitate to get in touch - we'll do whatever we can to fix it.

View the update logs.

Hello 👋
this is the third batch of possible improvements.

Missing rules:

This one is complete. 🎉 🎉

Rules that I would change:
Mostly config code style improvement so feel free to ignore.

• react-native/no-unused-styles error
• react-native/no-inline-styles error
• react-native/no-color-literals error
• react-native/no-raw-text 0 😄 actually I turned this off. turns out my config is not that strict after all 😂
• react-native/no-single-element-style-arrays error

The old cascading config is deprecated as of ESLint 9 which was just released as alpha. We need to migrate our ESLint configuration to the new, flat eslint config format.

Let's create an action that could be used on our repositories regardless if it's a frontend, backend, or any other (non-javascript) platform!

The idea is simple - to minify typos and silly grammar mistakes from our readmes, ADRs and any other markdown related content :)

@robertrossmann a quick question for you - do you think that this monorepo is a good place where to add our own github actions? Or shall we create them in a separate repository?

Standalone repository make sense in terms of easier usage.
This monorepo make sense to have everything together under one roof.

eslint-plugin-jsx-a11y

See control-has-associated-label docs and decide whether or not we should enable this rule. 🙏

The rule is part of v6.2.0 release.

eslint-plugin-react

A lot. See the release notes for v7.13 & v7.14.

eslint-plugin-react-hooks

Uhm, dunno? They do not provide a changelog for that package. 🤷‍♂ 🤦‍♂ The relevant changes are since v1.5.

Let's install Greenkeeper or a similar service to get an automatical update of dependencies.

For example, eslint-plugin-import is out of date. And it doesn't work well with @typescript-eslint/parser, which throws errors on types and interfaces for import/named rule

JSDoc rules in ESLint have been deprecated.

We might want to move to eslint-plugin-jsdoc via a new configuration file in the eslint-config-base ruleset (JSDoc validation might be useful in all JS environments, including Node, React, RN, and TypeScript).

The valid-jsdoc could be enabled for TypeScript but with a different configuration, especially the requireParamType should be disabled. Further modifications may be applied.

Reason 1 - The times of the Hungarian notation have passed

The main argument from I-prefix-for-interface supporters is that prefixing is helpful for immediately grokking (peeking) whether a type is an interface. A statement that prefix is helpful for immediately grokking (peeking) is an appeal to Hungarian notation. I prefix for interface name, C for class, A for abstract class, s for a string variable, c for const variable, i for integer variable. I agree that such name decoration can provide you type information without hovering mouse over identifier or navigating to type definition via a hot-key. This tiny benefit is outweighed by Hungarian notation disadvantages and other reasons mentioned below. Hungarian notation is not used in contemporary frameworks. C#, for example, has I prefix (and this the only prefix in C#) for interfaces due to historical reasons (COM). In retrospect one of the .NET architects (Brad Abrams) thinks it would have been better not using I prefix. TypeScript is COM-legacy-free thereby it shouldn't have an I-prefix-for-interface rule.

Reason 2 - I-prefix violates encapsulation principle

Let's assume you get some black-box. You get some type reference that allows you to interact with that box. You should not care if it is an interface or a class. You just use its interface part. Demanding to know what is it (interface, specific implementation or abstract class) is a violation of encapsulation.

Example: let's assume you need to fix API Design Myth: Interface as Contract in your code e.g. delete ICar interface and use Car base-class instead. Then you need to perform such replacement in all consumers. I-prefix leads to an implicit dependency of consumers on black-box implementation details.

Reason 3 - Protection from bad naming

Developers are lazy to think properly about names. Naming is one of the Two Hard Things in Computer Science. When a developer needs to extract an interface it is easy to just add the letter I to the class name and you get an interface name. Disallowing I prefix for interfaces forces developers to strain their brains to choose appropriate names for interfaces. Chosen names should be different not only in prefix but emphasize intent difference.

Abstraction case: you should not define an ICar interface and an associated Car class. Car is an abstraction and it should be the one used for the contract. Implementations should have descriptive, distinctive names e.g. SportsCar, SuvCar, HollowCar.

Good example: WpfeServerAutosuggestManager implements AutosuggestManager, FileBasedAutosuggestManager implements AutosuggestManager.

Bad example: AutosuggestManager implements IAutosuggestManager.

Reason 4 - Properly chosen names vaccinate you against API Design Myth: Interface as Contract.

In my practice, I met a lot of people that thoughtlessly duplicated interface part of a class in a separate interface having Car implements ICar naming scheme. Duplicating interface part of a class in separate interface type does not magically convert it into abstraction. You will still get concrete implementation but with duplicated interface part. If your abstraction is not so good, duplicating interface part will not improve it anyhow. Extracting abstraction is hard work.

All these reasons were presented by Stanislav Berkov
on StackOverFlow as a repetition from some of the important heads of Microsoft and Oracle about how using this pattern was a bad idea.

Hello 👋
this is the second batch of possible improvements.

Missing rules:

• react/button-has-type error
• react/no-children-prop error
• react/prefer-es6-class error (although probably not necessary anymore)
• react/prefer-read-only-props error (not sure how this works with typescript)
• react/prefer-stateless-function error
• react/sort-prop-types ? (I turned this off in my projects but if it works with typescript and is auto-fixable then it could be useful but I think we can ignore this rule)
• react/jsx-boolean-value error
• react/jsx-child-element-spacing error
• react/jsx-closing-tag-location error
• https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-wrap-multilines.md error

Rules that I would change:
Mostly config code style improvement so feel free to ignore.

• react/boolean-prop-naming error
• react/no-danger error
• react/no-deprecated error
• react/jsx-handler-names error
• react/jsx-key error
• react/jsx-no-target-blank error
• react/jsx-curly-brace-presence error
• react/jsx-fragments error
• react/jsx-no-useless-fragment error
• react/jsx-pascal-case error
• react/destructuring-assignment error
• react/jsx-closing-bracket-location error
• react/jsx-first-prop-new-line error
• react/jsx-indent error
• react/jsx-tag-spacing error
• react/self-closing-comp error

Lerna has been marked as no longer maintained, we need to move away to a different monorepo management tool. ⚠️

@typescript-eslint/explicit-function-return-type enforces every function defines a returning type. Most of the time, though, TypeScript can infer the returning type from the return value. This rules causes many redundant and unnecessary code.

Ref.: typescript-eslint/typescript-eslint#434

The rules for this package extend config-conventional from commitlint authors. It turns out that our Git guidelines state that we should Capitalize the subject line. config-conventional, on the other hand, sets this as never.

The result is: if we follow our convention, having commitlint-config set, we get stuck in a commit-msg hook 🤷.

We should consider picking either on or the other.

When you commit with Commitizen, you'll be prompted to fill out any required commit fields at commit time. No more waiting until later for a git commit hook to run and reject your commit (though that can still be helpful). No more digging through CONTRIBUTING.md to find what the preferred format is. Get instant feedback on your commit message formatting and be prompted for required fields.
https://github.com/commitizen/cz-cli

• Can be used as prepare-commit-msg
• We can apply it both for this repository and as commitlint package's recommended extension

We need to use @typescript-eslint/no-unused-expressions instead of ESlint no-unused-expressions , because it can't handle optional chaining.

This code emits warning Expected an assignment or function call and instead saw an expression.

fn?.('hello')

Related:
typescript-eslint/typescript-eslint#1241 (comment)

Hello 👋
last update.

Missing rules:

• @typescript-eslint/ban-ts-ignore warn (funny how I set everything to error but I leave this as warn isn't it? 😅🙉)
• @typescript-eslint/camelcase warn (I would love to set this as an error but apollo code gen...)
• @typescript-eslint/class-name-casing error
• generic-type-naming error [major] (this one is deprecated I will investigate naming-convention but this is one of my favorite finds 🙂)
• @typescript-eslint/no-magic-numbers warn (I turned this off because it was impossible to use it in existing projects it might be interesting to set this to warn for new projects 🙉 )

Missing plugins:

• eslint-plugin-unused-imports (this thing is amazing 👌)

Missing rules from missing plugins:

• unused-imports/no-unused-imports 0
• unused-imports/no-unused-imports-ts 2
• unused-imports/no-unused-vars
• unused-imports/no-unused-vars-ts

Rules that I would change:

• @typescript-eslint/array-type error, array
• @typescript-eslint/await-thenable error
• @typescript-eslint/no-floating-promises error (very useful rule that forces to catch errors or use await for promises)
• @typescript-eslint/no-for-in-array 0
• @typescript-eslint/no-misused-new error
• no-misused-promises error
• @typescript-eslint/no-parameter-properties 0 (important in nest.js)
• @typescript-eslint/no-this-alias error
• @typescript-eslint/no-unnecessary-qualifier error
• @typescript-eslint/no-unnecessary-type-assertion error
• @typescript-eslint/no-unused-vars 0 (I think this needs to be off for the plugin to work)
• @typescript-eslint/promise-function-async 0
• @typescript-eslint/require-await error 🙉
• @typescript-eslint/restrict-template-expressions 0
• @typescript-eslint/prefer-function-type error
• @typescript-eslint/prefer-includes error
• @typescript-eslint/prefer-nullish-coalescing error (autofixable)
• @typescript-eslint/prefer-optional-chain error (autofixable)
• @typescript-eslint/prefer-string-starts-ends-with error

For fun:

• @typescript-eslint/no-explicit-any 0 (I have this one off 😅)
• explicit-function-return-type 0 (actually I turned this off 🙂)
• @typescript-eslint/explicit-member-accessibility 0 (I turned this off as well 😮)

Two new errors for me. 😂

ESLint is deprecating its formatting rules. We need to find an alternative.

https://github.com/loeffel-io/ls-lint

Written in Go but clearly aimed at JS/front-end dev use cases. ls-lint provides a way to enforce rules for file naming and directory structures.

https://github.com/danger/danger-js

You can:

• Enforce CHANGELOGs
• Enforce links to Trello/JIRA in PR/MR bodies
• Enforce using descriptive labels
• Look out for common anti-patterns
• Highlight interesting build artifacts
• Give warnings when specific files change

There is also this loveliness: https://github.com/danger/peril

https://github.com/airbnb/javascript/blob/master/packages/eslint-config-airbnb-base/rules/variables.js#L18-L31

confusing-browser-globals looks like a great list maintained in Facebook's CRA.

I'm using @strv/eslint-config-react together with @strv/eslint-config-typescript. This combination is giving me the following error:

warning  JSX not allowed in files with extension '.tsx' react/jsx-filename-extension

For Typescript we want to override this rule to allow tsx extensions but I'm not sure if we should add it directly to the typescript ruleset as its not relevant for Node.js.

Maybe we can add a @strv/eslint-config-typescript/react?

Since v10 css-in-js is builtin in Stylelint

styled-components/stylelint-processor-styled-components#278

The stylelint tool supports linting raw CSS and Less files. We should make use of it!

Let's rename stylelint-config-styled-components to just stylelint-config and add a generic configuration for CSS and Less, and then add a styled-components ruleset which would enable support for style-in-js patterns. 👍

Hello 👋
a couple of weeks ago (it feels like a long time now 😅) we discussed what we can do to improve STRV code quality tools. This is my first follow up about eslint-config-base.

I will share a similar update about node, react, react-native, and typescript this week.

Missing rules:
I would highly recommend taking a look at everything labeled as [major]. 🙂

• no-dupe-else-if error
• no-import-assign error
• no-prototype-builtins error
• no-setter-return ? (right now I have it set as error because I don't do OOP otherwise I would turn it off so that I can write fluent setters)
• block-scoped-var error [major]
• complexity (useful but up to discussion how strict we want to have it [‘error’, 5 to 7 seems like good values])
• grouped-accessor-pairs error
• no-alert ? error
• no-constructor-return error
• no-div-regex error
• no-empty-function ? (not having it at all seems like a good idea)
• no-eq-null error
• no-implicit-globals error [major]
• no-invalid-this error
• no-param-reassign error [major]
• no-return-await ? error
• no-useless-catch error [major]
• no-void ? error (I don't use it so I turned it off)
• prefer-regex-literals error
• radix error [major]
• init-declarations ? (I have this off)
• capitalized-comments (I have this off)
• max-lines ? up to discussion how strict we can make this
• max-lines-per-function ? up to discussion how strict we can make this
• no-bitwise error (personally I would motivate people NOT to use bitwise operators because of what I have seen in Otter.ai)
• no-multi-assign error [kind of major for me]
• no-negated-condition error [major]
• no-plusplus error [major]
• wrap-regex error
• no-useless-rename erorr
• prefer-destructuring error [major]
• import/no-default-export error [major]

Missing plugins:

• absolute-import
• unused-imports [major]

Missing rules from missing plugins:

• absolute-import/no-relative-path warning [major]
• unused-imports/no-unused-imports error [major]
• unused-imports/no-unused-vars error [major]

Rules that can be more strict:

• import-order see comment bellow
• no-loop-func error
• no-unused-vars this can be off when we use unused-imports plugin
• no-new-object error [major]
• no-template-curly-in-string error
• no-unmodified-loop-condition error
• no-useless-concat error
• no-useless-return error [major]
• prefer-spread error [major]
• require-await error (this fall in the group with no-return-await I saw async keyword being misused, essentially it changes the function to promise for no reason I believe that is a bad practice)
• vars-on-top error [major]
• no-console error [major]
• func-names error
• no-undefined 0 (actually I turned this off as I prefer undefined over null)
• symbol-description error
• import/no-duplicates error [major]

Rules that I would change:

• pretty much everything can be set as error
• if we want to be consistent then code style issues shouldn't make it to GitHub
• I see no value in warnings. When we as a team decide which rules are important we should follow these rules. 🙂
• Personally I use warn only for rules that I know I shouldn't break but sometimes I have to (no-console, ban-ts-ignore, camelcase [because of graphql code gen], ...)

https://github.com/secretlint/secretlint

There is snyk, there is this little guy: https://github.com/lirantal/is-website-vulnerable

Should we consider adding these tools to this monorepo?

So far, the recommended approach for enabling both @strv/eslint-config-react and @strv/eslint-config-typescript is that:

// .eslintrc.js
module.exports = {
  "extends": [
    "@strv/eslint-config-react",
    "@strv/eslint-config-typescript"
  ]
}

These base configurations do conflict in some aspects. I'm writing this issue while trying to properly configure code-quality-tools in a frontend project, and will add here these eventual conflicts and their solutions.

Hello 👋
second to the last update. 🎉

I will share a similar update about node, react, react-native, and typescript this week.

Missing rules:

• node/no-extraneous-import error
• node/no-extraneous-require error
• node/no-unpublished-import error
• node/no-unpublished-require error
• node/exports-style error

Prefer global:

I have all set as an error but I these rules are not useful and it doesn't make much sense using them. Although it might be interesting to set them as error, never. 🙂

• node/prefer-global/buffer
• node/prefer-global/console
• node/prefer-global/process
• node/prefer-global/text-decoder
• node/prefer-global/text-encoder
• node/prefer-global/url-search-params
• node/prefer-global/url

Prefer promise:

• node/prefer-promises/dns error
• node/prefer-promises/fs error

Missing plugins:

• eslint-plugin-security-node

Missing rules from missing plugins:
I have some warnings here because I was doing unconventional things in the create-opinionated-app generator. I don't do BE full time so it will be up to someone else to decide what can be disabled and what can be left as a warning.

• security/detect-buffer-noassert error
• security/detect-child-process error
• security/detect-disable-mustache-escape error
• security/detect-eval-with-expression error
• security/detect-new-buffer error
• security/detect-no-csrf-before-method-override error
• security/detect-non-literal-fs-filename warning
• security/detect-non-literal-regexp error
• security/detect-non-literal-require warning
• security/detect-object-injection warning
• security/detect-possible-timing-attacks error
• security/detect-pseudoRandomBytes error
• security/detect-unsafe-regex error

Rules that I would change:

• node/no-callback-literal error
• node/no-deprecated-api error

https://github.com/Rantanen/eslint-plugin-xss

iOS guys are using https://fastlane.tools/ :) I think before we jump into CLI development, we should examine, what is the correct way, so it could be used universally on any platform.

See 47bfb8c for details about why we should probably work on improving the ruleset authoring experience. Boogs like these should not be possible to introduce into a package used by the whole company. 😱 🤦‍♂️

https://eslint.org/blog/2021/10/eslint-v8.0.0-released 🙃

• add https://github.com/mdx-js/eslint-mdx
• how to integrate with our rules (maybe create a preset)