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.

Sign up for GitHub

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

chore(repo): setting up the base for migrating to eslint #12923

Merged
merged 9 commits into from
Jan 31, 2024
Merged

chore(repo): setting up the base for migrating to eslint #12923

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e20782e
chore(repo): use typescript 5.0.2 across workspace
HuiSF Jan 25, 2024
fa0423d
chore(repo): refactor tsconfig hierarchy
HuiSF Jan 26, 2024
8b3b204
chore(repo): promote rollup dependencies into workspace
HuiSF Jan 29, 2024
dd497d0
chore(repo): add build:watch using rollup
HuiSF Jan 29, 2024
872d825
chore(repo): setup eslint
HuiSF Jan 29, 2024
5b7a1e9
chore(adapter-nextjs): migrate to eslint
HuiSF Jan 29, 2024
ca85c04
fix(repo): test-github-actions using js-yaml already removed API
HuiSF Jan 29, 2024
c6edaa2
chore(repo): add Code Spell Checker to recommended extension list
HuiSF Jan 29, 2024
d047026
chore(repo): add formatOn actions to formattingToggle controlables
HuiSF Jan 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
190 changes: 190 additions & 0 deletions .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
/** @type {import("eslint").ESLint.ConfigData}*/
module.exports = {
root: true,
parser: '@typescript-eslint/parser',
parserOptions: {
project: './tsconfig.json',
},
extends: [
'eslint:recommended',
'standard',
'plugin:import/errors',
'plugin:import/recommended',
'plugin:import/typescript',
'plugin:@typescript-eslint/stylistic',
'plugin:@typescript-eslint/recommended',
calebpollman marked this conversation as resolved.
Show resolved Hide resolved
"plugin:prettier/recommended",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be careful with usage of "plugin:prettier/recommended". Mixing linting with formatting can lead to undesired conflicts

],
plugins: [
'@stylistic',
'@typescript-eslint',
'unused-imports',
'import',
'jsdoc',
],
env: {
es6: true,
node: true,
},
ignorePatterns: [
'dist',
'node_modules',
'.eslintrc.*',
'rollup',
'rollup.config.*',
'setupTests.ts',
'jest.setup.*',
'jest.config.*',
// temporarily disable lint on __tests__
'__tests__',
// will enable lint by packages
// 'adapter-nextjs',
jimblanc marked this conversation as resolved.
Show resolved Hide resolved
'analytics',
'api',
'api-graphql',
'auth',
'aws-amplify',
'core',
'datastore',
'datastore-storage-adapter',
'geo',
'interactions',
'notifications',
'predictions',
'pubsub',
'react-native',
'rtn-push-notification',
'rtn-web-browser',
'storage',
],
rules: {
camelcase: [
'error',
{
allow: [
'graphql_headers',
// exceptions for the legacy config
/^(aws_|amazon_)/,
'access_key',
'secret_key',
'session_token',
// exceptions for the auth package
'redirect_uri',
'response_type',
'client_id',
'identity_provider',
'code_challenge',
'code_challenge_method',
'grant_type',
'code_verifier',
'logout_uri',
'id_token',
'access_token',
'token_type',
'expires_in',
'error_description',
// exceptions for the notifications package
'campaign_id',
'delivery_type',
'treatment_id',
'campaign_activity_id',
],
},
],
'import/no-deprecated': 'warn',
'import/no-empty-named-blocks': 'error',
'import/no-mutable-exports': 'error',
'import/no-relative-packages': 'error',
'import/newline-after-import': 'error',
'import/order': ['error', { 'newlines-between': 'always' }],
'no-eval': 'error',
'no-param-reassign': 'error',
'no-shadow': 'off',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely an opinion, but turning off no-shadow can lead to code that is difficult to grok as a reader

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just saw that this is turned off in favor of the @typescript-eslint version 😅

For future maintainers, might be helpful to note which rules are superseded by corresponding @typescript-eslint rules

'no-use-before-define': 'off',
'no-useless-constructor': 'off',
'no-trailing-spaces': 'error',
'no-return-await': 'error',
'object-shorthand': 'error',
'prefer-destructuring': 'off',
'promise/catch-or-return': [
'error',
{ terminationMethod: ['then', 'catch', 'asCallback', 'finally'] },
],
'space-before-function-paren': 'off',
'sort-imports': ['error', { ignoreDeclarationSort: true }],
'unused-imports/no-unused-imports': 'error',
'unused-imports/no-unused-vars': [
'error',
{
vars: 'all',
varsIgnorePattern: '^_',
args: 'after-used',
argsIgnorePattern: '^_',
},
],
'valid-typeof': ['error', { requireStringLiterals: false }],
'@stylistic/comma-dangle': [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reads as something that Prettier could just handle?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prettier handles this (when it actually reformats), this rule is for showing errors with the eslint in details.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels extraneous IMO but do not have a strong opinion

'error',
{
arrays: 'always-multiline',
objects: 'always-multiline',
imports: 'always-multiline',
exports: 'always-multiline',
functions: 'always-multiline',
enums: 'always-multiline',
generics: 'always-multiline',
tuples: 'always-multiline',
},
],
'@stylistic/function-call-argument-newline': ['error', 'consistent'],
'@stylistic/indent': 'off',
'@stylistic/max-len': [
'error',
{
code: 120,
ignoreComments: true,
ignoreUrls: true,
ignoreStrings: true,
ignoreTemplateLiterals: true,
ignoreRegExpLiterals: true,
},
],
'@stylistic/padding-line-between-statements': [
'error',
{ blankLine: 'always', prev: '*', next: 'return' },
],
'@typescript-eslint/method-signature-style': ['error', 'method'],
'@typescript-eslint/no-confusing-void-expression': 'error',
'@typescript-eslint/no-explicit-any': 'off',
'@typescript-eslint/no-namespace': ['error', { allowDeclarations: true }],
'@typescript-eslint/no-shadow': 'error',
'@typescript-eslint/no-var-requires': 'off',
'@typescript-eslint/no-unused-vars': 'off',
'@typescript-eslint/no-use-before-define': [
'error',
{ functions: false, variables: false, classes: false },
],
'@typescript-eslint/no-useless-constructor': 'error',
'@typescript-eslint/prefer-destructuring': [
'error',
{ object: true, array: false },
],
'@typescript-eslint/space-before-function-paren': [
'error',
{ anonymous: 'never', named: 'never', asyncArrow: 'always' },
],
'jsdoc/no-undefined-types': 1,
},
settings: {
'import/parsers': {
'@typescript-eslint/parser': ['.ts', '.tsx'],
},
'import/resolver': {
typescript: {
alwaysTryTypes: true,
project: ['packages/*/tsconfig.json', 'tsconfig.json'],
},
},
'import/ignore': ['react-native'],
},
};
1 change: 0 additions & 1 deletion .prettierignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ docs
package.json
yarn.lock
package-lock.json
.eslintrc.js
www
.stencil
PULL_REQUEST_TEMPLATE.md
Expand Down
4 changes: 3 additions & 1 deletion .vscode/extensions.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
// List of extensions which should be recommended for users of this workspace.
"recommendations": [
"esbenp.prettier-vscode",
"tombonnike.vscode-status-bar-format-toggle"
"tombonnike.vscode-status-bar-format-toggle",
"dbaeumer.vscode-eslint",
"streetsidesoftware.code-spell-checker"
],
// List of extensions recommended by VS Code that should not be recommended for users of this workspace.
"unwantedRecommendations": []
Expand Down
13 changes: 7 additions & 6 deletions .vscode/settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
{
"[typescript]": {
"editor.defaultFormatter": "esbenp.prettier-vscode"
},
"editor.defaultFormatter": "esbenp.prettier-vscode",
"editor.detectIndentation": false,
"editor.formatOnSave": true,
"editor.insertSpaces": false,
"editor.tabSize": 4,
"prettier.requireConfig": true,
"typescript.tsdk": "node_modules/typescript/lib"
"typescript.tsdk": "node_modules/typescript/lib",
"formattingToggle.affects": [
"editor.codeActionsOnSave.source.fixAll.eslint",
"editor.formatOnPaste",
"editor.formatOnSave",
"editor.formatOnType"
]
Comment on lines +7 to +12
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels a bit restrictive/opinionated, what is the context of this addition?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not restrictive, IMO, but gives the most freedom.

  • reformat on save is not enabled by default
  • ppl who are using vscode can install the recommended extension formatting toggle to enable reformat on save (this is the existing set up prior to this migration)
  • for ppl who are not using auto reformat at all, they will catch formatting issues when run yarn test

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, misread these as enforced

}
30 changes: 19 additions & 11 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,13 @@
"test:tsc-compliance": "yarn workspace tsc-compliance-test test:compliance",
"docs": "typedoc packages/**/src --name amplify-js --hideGenerator --excludePrivate --ignoreCompilerErrors --mode file --out docs/api --theme docs/amplify-theme/typedoc/ --readme README.md",
"build": "lerna run build --stream && yarn test:duplicates",
"build:watch": "concurrently 'lerna run build:cjs:watch --parallel' 'lerna run build:esm:watch --parallel' --raw",
"build:esm:watch": "lerna run build:esm:watch --parallel",
"build:cjs:watch": "lerna run build:cjs:watch --parallel",
"build:watch": "concurrently 'lerna run build:watch --parallel' --raw",
"build:client-types": "cd scripts/dts-bundler && yarn && yarn run build",
"clean": "lerna run clean --parallel",
"clean:size": "lerna run clean:size --parallel",
"format": "lerna run format",
"lint": "lerna run lint",
"lint:fix": "lerna run lint:fix",
"lint:license": "license-check-and-add add -f license_config.json",
"link-all": "yarn unlink-all && lerna exec --no-bail --parallel yarn link",
"unlink-all": "lerna exec --no-bail --parallel -- yarn unlink; exit 0",
Expand Down Expand Up @@ -81,43 +80,52 @@
"@babel/preset-env": "^7.0.0",
"@babel/preset-react": "^7.0.0",
"@lerna/legacy-package-management": "^7.4.2",
"@rollup/plugin-typescript": "^11.1.6",
"@size-limit/dual-publish": "^8.1.0",
"@size-limit/file": "^8.1.0",
"@size-limit/webpack": "^8.1.0",
"@size-limit/webpack-why": "^8.1.0",
"@stylistic/eslint-plugin": "^1.5.4",
"@types/jest": "^29.5.8",
"@types/lodash": "4.14.182",
"@types/node": "^8.9.5",
"@types/puppeteer": "1.3.0",
"@typescript-eslint/eslint-plugin": "^6.20.0",
"@typescript-eslint/parser": "^6.20.0",
"babel-loader": "^8.3.0",
"eslint": "^8.56.0",
jimblanc marked this conversation as resolved.
Show resolved Hide resolved
"eslint-config-prettier": "^9.1.0",
"eslint-config-standard": "^17.1.0",
"eslint-import-resolver-typescript": "^3.6.1",
"eslint-plugin-import": "^2.29.1",
"eslint-plugin-jsdoc": "^48.0.4",
"eslint-plugin-n": "^16.6.2",
"eslint-plugin-prettier": "^5.1.3",
"eslint-plugin-promise": "^6.1.1",
"eslint-plugin-unused-imports": "^3.0.0",
"glob": "^10.3.10",
"jest": "^29.7.0",
"jest-environment-jsdom": "^29.7.0",
"json-loader": "^0.5.7",
"lerna": "^7.4.2",
"license-check-and-add": "^4.0.5",
"mkdirp": "^3.0.1",
"prettier": "^3.1.0",
"rimraf": "^2.6.2",
"rollup": "^0.67.4",
"rollup-plugin-commonjs": "^9.2.0",
"rollup-plugin-json": "^3.1.0",
"rollup-plugin-node-resolve": "^4.0.0",
"rollup-plugin-sourcemaps": "^0.4.2",
"rollup-plugin-typescript": "^1.0.0",
"rollup": "^4.9.6",
"size-limit": "^8.1.0",
"terser-webpack-plugin": "^5.3.6",
"ts-jest": "^29.1.1",
"ts-loader": "^9.4.3",
"tslint": "^5.7.0",
"tslint-config-airbnb": "^5.8.0",
"typedoc": "^0.17.0",
"typescript": "^4.3.5",
"typescript": "~5.0.2",
"typescript-coverage-report": "^0.6.4",
"uuid-validate": "^0.0.3",
"webpack": "^5.75.0",
"webpack-bundle-analyzer": "^4.7.0",
"webpack-cli": "^5.0.0",
"winston": "^3.2.1",
"wml": "0.0.83"
},
"resolutions": {
Expand Down
11 changes: 4 additions & 7 deletions packages/adapter-nextjs/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,13 @@
"cookie": "0.5.0"
},
"devDependencies": {
"@rollup/plugin-typescript": "11.1.5",
"@types/cookie": "0.5.1",
"@types/node": "^20.3.1",
"@types/react": "^18.2.13",
"@types/react-dom": "^18.2.6",
"aws-amplify": "6.0.13",
"jest-fetch-mock": "3.0.3",
"next": ">= 13.5.0 < 15.0.0",
"rollup": "3.29.4",
"typescript": "5.0.2"
"next": ">= 13.5.0 < 15.0.0"
},
"publishConfig": {
"access": "public"
Expand Down Expand Up @@ -62,13 +59,13 @@
"scripts": {
"build": "npm run clean && npm run build:esm-cjs",
"build-with-test": "npm test && npm run build",
"build:cjs:watch": "rimraf dist/cjs && tsc -m commonjs --outDir dist/cjs --watch",
"build:esm-cjs": "rollup -c rollup.config.mjs",
"build:esm:watch": "rimraf dist/esm && tsc -m esnext --outDir dist/esm --watch",
"build:watch": "npm run build:esm-cjs -- --watch",
"clean": "npm run clean:size && rimraf dist",
"clean:size": "rimraf dual-publish-tmp tmp*",
"format": "echo \"Not implemented\"",
"lint": "tslint 'src/**/*.ts' && npm run ts-coverage",
"lint": "eslint '**/*.{ts,tsx}' && npm run ts-coverage",
"lint:fix": "eslint '**/*.{ts,tsx}' --fix",
"test": "npm run lint && jest -w 1 --coverage --logHeapUsage",
"ts-coverage": "typescript-coverage-report -p ./tsconfig.build.json -t 90.31"
}
Expand Down
14 changes: 2 additions & 12 deletions packages/adapter-nextjs/rollup.config.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,13 @@ const config = defineConfig([
{
input: input,
output: cjsOutput,
plugins: [
typescript({
...cjsTSOptions,
tsconfig: 'tsconfig.build.json',
}),
],
plugins: [typescript(cjsTSOptions)],
},
// ESM config
{
input: input,
output: esmOutput,
plugins: [
typescript({
...esmTSOptions,
tsconfig: 'tsconfig.build.json',
}),
],
plugins: [typescript(esmTSOptions)],
},
]);

Expand Down
1 change: 1 addition & 0 deletions packages/adapter-nextjs/src/createServerRunner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

import { ResourcesConfig } from 'aws-amplify';

import { createRunWithAmplifyServerContext, getAmplifyConfig } from './utils';
import { NextServer } from './types';

Expand Down
Loading
Loading