cdk-monitoring-constructs icon indicating copy to clipboard operation
cdk-monitoring-constructs copied to clipboard

[Infra] Improve eslint/prettier setup

Open voho opened this issue 2 years ago • 5 comments

Feature scope

build

Describe your suggested feature

I think our Projen setup is inconsistent in how it applies eslint/prettier rules. Seems to me that there are two conflicting definitions, or something like that - for example, the lines are wrapped eagerly, which is not what I would expect. I think we have to find a way to specify the rules on single place and then make all the tools use the same config.

Found this example:

https://github.com/p6m7g8/awesome-projen/blob/main/.eslintrc.json

voho avatar Mar 30 '22 07:03 voho

The formatting might have been happening eagerly because we have source.fixAll.eslint to true on save in the VSCode settings for the project: https://github.com/cdklabs/cdk-monitoring-constructs/blob/main/.vscode/settings.json .

Is this not what is expected? What are the inconsistencies that you have observed?

ayush987goyal avatar Mar 30 '22 08:03 ayush987goyal

I think the line wrapping is very tight, although we did not observe the same in the internal package. I am actually not sure if we use eslint or prettier for formatting, and where the SSOT is, since we have config for both :).

voho avatar Apr 01 '22 08:04 voho

One addition, would be to improve the .gitignore as when trying to build all the resultant files in the test directory were not being ignored. Maybe using a gitignore generator would be helpful?

rahul0705 avatar Apr 04 '22 15:04 rahul0705

For reference, the internal ESLint configuration is currently set to:

module.exports = {
    env: {
        es2021: true
    },
    extends: ["eslint:recommended", "plugin:@typescript-eslint/recommended", "plugin:prettier/recommended"],
    parser: "@typescript-eslint/parser",
    parserOptions: {
        ecmaVersion: 2018,
        sourceType: "module"
    },
    plugins: ["@typescript-eslint", "unused-imports"],
    rules: {
        "unused-imports/no-unused-imports": "error",
        "no-case-declarations": "off",

        "@typescript-eslint/no-explicit-any": "off",
        "@typescript-eslint/explicit-module-boundary-types": "off",
        "@typescript-eslint/no-unused-vars": ["error", {argsIgnorePattern: "^_"}],

        "prettier/prettier": [
            "error",
            {
                arrowParens: "always",
                bracketSpacing: false,
                printWidth: 140,
                semi: true,
                tabWidth: 4,
                trailingComma: "all",
            }
        ]
    }
};

But we opted to stick with projen's default configs when setting up this repo to avoid some weird mismatches/churn.

There probably isn't much benefit to necessarily matching 1-1 since the codebases are fully separate anyway, but the line wrapping would likely be addressed by setting Prettier's printWidth setting.

echeung-amzn avatar Apr 04 '22 16:04 echeung-amzn

I'll tackle this at some point next week.

echeung-amzn avatar Apr 07 '22 21:04 echeung-amzn