Update eslint configuration #1129

wjhsf · 2023-04-13T21:09:07Z

Lots of changes! Mostly linter fixes, some config changes. Most of the linter fixes are trivial/inconsequential, but some required mild refactoring. See #1130 for a subset of these changes, hopefully easier to scroll through!

Changes:

Added TypeScript checks, accessibility checks, jest checks, and (more) React checks!
Lots and lots of linter fixes!
Introduced circular dev dependencies: pwa-kit-dev/internal-lib-build and pwa-kit-dev/pwa-kit-runtime
Removed internal-lib-build lint, as the intention is to have pwa-kit-dev lint as the single source of truth.
Header config now lives exclusively in the monorepo root, rather than shipping pwa-kit-dev with a dependency it doesn't actually use.
Added proper typescript support to pwa-kit-dev, since it has one TypeScript file.
Improved TypeScript support in template-typescript-minimal and test-commerce-sdk-react.
Possibly more? I reviewed the changes to make this list, but I did this work in bursts over a few weeks, so I might have missed something.

NOTE: This is not the end of the eslint work! I'll do a follow-up PR for some cleanup like fixing the circular deps. The scope of this PR is just to introduce the new linter rules, so that the follow up work doesn't get lost in the sea of changes.

Description

Types of Changes

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Documentation update
Breaking change (could cause existing functionality to not work as expected)
Other changes (non-breaking changes that does not fit any of the above)

Breaking changes include:

Removing a public function or component or prop

Adding a required argument to a function

Changing the data type of a function parameter or return value

Adding a new peer dependency to package.json

Changes

(change1)

How to Test-Drive This PR

(step1)

Checklists

General

Changes are covered by test cases
CHANGELOG.md updated with a short description of changes (not required for documentation updates)

Accessibility Compliance

You must check off all items in one of the follow two lists:

There are no changes to UI

or...

Changes were tested with a Screen Reader (iOS VoiceOver or Android Talkback) and had no issues
Changes comply with WCAG 2.0 guidelines levels A and AA
Changes to common UI patterns and interactions comply with WAI-ARIA best practices

Localization

Changes include a UI text update in the Retail React App (which requires translation)

The header is only enforced inside the monorepo.

This reverts commit 0a21099.

…l-lib-build.

wjhsf · 2023-04-14T18:03:26Z

scripts/check-dependencies.js

+
+// For dependencies that we want to use the same version in all packages,
+// use this package as the source of truth for the version to use
+const COMMON_DEPS_SOURCE_OF_TRUTH = 'template-retail-react-app'


Turns out internal-lib-build doesnt' actually use cross-env, so I changed the source of truth to template-retail-react-app.

This reverts commit 491a4ce.

This reverts commit bbee379.

…tions.

wjhsf · 2023-04-17T20:52:42Z

packages/commerce-sdk-react/src/hooks/ShopperLogin/mutation.test.ts

@@ -60,7 +61,8 @@ const TOKEN_RESPONSE: ShopperLoginTypes.TokenResponse = {
    id_token: 'id_token',
    refresh_token: 'refresh_tone',
    token_type: 'token_type',
-    usid: 'usid'
+    usid: 'usid',
+    idp_access_token: 'idp_access_token'


Just some bug fixes here, I think due to the updated isomorphic SDK?

wjhsf · 2023-04-17T20:54:35Z

packages/pwa-kit-dev/bin/pwa-kit-dev.js

@@ -332,11 +333,8 @@ const main = async () => {
        .option('--fix', 'Try and fix errors (default: false)')
        .action(async (path, {fix}) => {
            const eslint = p.join(require.resolve('eslint'), '..', '..', '..', '.bin', 'eslint')
-            const eslintConfig = p.join(__dirname, '..', 'configs', 'eslint', 'eslint-config.js')


❗ This is part of the separate story to fix pwa-kit-dev lint. I needed to do this because I've introduced multiple configs. There's more work to do, though, to finish the other story.

wjhsf · 2023-04-17T20:57:39Z

packages/pwa-kit-dev/src/configs/eslint/README.md

@@ -0,0 +1,21 @@
+# `pwa-kit-dev` ESLint configuations


❗ This README explains the three different configs we have. We currently use recommended (default) and no-react (self-explanatory?). As we adopt TypeScript more fully, I anticipate that our projects will migrate towards using safe-types.

bfeister · 2023-04-17T21:24:48Z

packages/pwa-kit-dev/tsconfig.json

+
+    /* Projects */
+    // "incremental": true,                              /* Save .tsbuildinfo files to allow for incremental compilation of projects. */
+    // "composite": true,                                /* Enable constraints that allow a TypeScript project to be used with project references. */
+    // "tsBuildInfoFile": "./.tsbuildinfo",              /* Specify the path to .tsbuildinfo incremental compilation file. */
+    // "disableSourceOfProjectReferenceRedirect": true,  /* Disable preferring source files instead of declaration files when referencing composite projects. */
+    // "disableSolutionSearching": true,                 /* Opt a project out of multi-project reference checking when editing. */
+    // "disableReferencedProjectLoad": true,             /* Reduce the number of projects loaded automatically by TypeScript. */
+
+    /* Language and Environment */
+    "target": "es2016",                                  /* Set the JavaScript language version for emitted JavaScript and include compatible library declarations. */
+    // "lib": [],                                        /* Specify a set of bundled library declaration files that describe the target runtime environment. */
+    // "jsx": "preserve",                                /* Specify what JSX code is generated. */
+    // "experimentalDecorators": true,                   /* Enable experimental support for TC39 stage 2 draft decorators. */
+    // "emitDecoratorMetadata": true,                    /* Emit design-type metadata for decorated declarations in source files. */
+    // "jsxFactory": "",                                 /* Specify the JSX factory function used when targeting React JSX emit, e.g. 'React.createElement' or 'h'. */
+    // "jsxFragmentFactory": "",                         /* Specify the JSX Fragment reference used for fragments when targeting React JSX emit e.g. 'React.Fragment' or 'Fragment'. */
+    // "jsxImportSource": "",                            /* Specify module specifier used to import the JSX factory functions when using 'jsx: react-jsx*'. */
+    // "reactNamespace": "",                             /* Specify the object invoked for 'createElement'. This only applies when targeting 'react' JSX emit. */
+    // "noLib": true,                                    /* Disable including any library files, including the default lib.d.ts. */
+    // "useDefineForClassFields": true,                  /* Emit ECMAScript-standard-compliant class fields. */
+    // "moduleDetection": "auto",                        /* Control what method is used to detect module-format JS files. */
+
+    /* Modules */
+    "module": "commonjs",                                /* Specify what module code is generated. */
+    // "rootDir": "./",                                  /* Specify the root folder within your source files. */
+    // "moduleResolution": "node",                       /* Specify how TypeScript looks up a file from a given module specifier. */
+    // "baseUrl": "./",                                  /* Specify the base directory to resolve non-relative module names. */
+    // "paths": {},                                      /* Specify a set of entries that re-map imports to additional lookup locations. */
+    // "rootDirs": [],                                   /* Allow multiple folders to be treated as one when resolving modules. */
+    // "typeRoots": [],                                  /* Specify multiple folders that act like './node_modules/@types'. */
+    // "types": [],                                      /* Specify type package names to be included without being referenced in a source file. */
+    // "allowUmdGlobalAccess": true,                     /* Allow accessing UMD globals from modules. */
+    // "moduleSuffixes": [],                             /* List of file name suffixes to search when resolving a module. */
+    // "resolveJsonModule": true,                        /* Enable importing .json files. */
+    // "noResolve": true,                                /* Disallow 'import's, 'require's or '<reference>'s from expanding the number of files TypeScript should add to a project. */
+
+    /* JavaScript Support */
+    // "allowJs": true,                                  /* Allow JavaScript files to be a part of your program. Use the 'checkJS' option to get errors from these files. */
+    // "checkJs": true,                                  /* Enable error reporting in type-checked JavaScript files. */
+    // "maxNodeModuleJsDepth": 1,                        /* Specify the maximum folder depth used for checking JavaScript files from 'node_modules'. Only applicable with 'allowJs'. */
+
+    /* Emit */
+    // "declaration": true,                              /* Generate .d.ts files from TypeScript and JavaScript files in your project. */
+    // "declarationMap": true,                           /* Create sourcemaps for d.ts files. */
+    // "emitDeclarationOnly": true,                      /* Only output d.ts files and not JavaScript files. */
+    // "sourceMap": true,                                /* Create source map files for emitted JavaScript files. */
+    // "outFile": "./",                                  /* Specify a file that bundles all outputs into one JavaScript file. If 'declaration' is true, also designates a file that bundles all .d.ts output. */
+    // "outDir": "./",                                   /* Specify an output folder for all emitted files. */
+    // "removeComments": true,                           /* Disable emitting comments. */
+    // "noEmit": true,                                   /* Disable emitting files from a compilation. */
+    // "importHelpers": true,                            /* Allow importing helper functions from tslib once per project, instead of including them per-file. */
+    // "importsNotUsedAsValues": "remove",               /* Specify emit/checking behavior for imports that are only used for types. */
+    // "downlevelIteration": true,                       /* Emit more compliant, but verbose and less performant JavaScript for iteration. */
+    // "sourceRoot": "",                                 /* Specify the root path for debuggers to find the reference source code. */
+    // "mapRoot": "",                                    /* Specify the location where debugger should locate map files instead of generated locations. */
+    // "inlineSourceMap": true,                          /* Include sourcemap files inside the emitted JavaScript. */
+    // "inlineSources": true,                            /* Include source code in the sourcemaps inside the emitted JavaScript. */
+    // "emitBOM": true,                                  /* Emit a UTF-8 Byte Order Mark (BOM) in the beginning of output files. */
+    // "newLine": "crlf",                                /* Set the newline character for emitting files. */
+    // "stripInternal": true,                            /* Disable emitting declarations that have '@internal' in their JSDoc comments. */
+    // "noEmitHelpers": true,                            /* Disable generating custom helper functions like '__extends' in compiled output. */
+    // "noEmitOnError": true,                            /* Disable emitting files if any type checking errors are reported. */
+    // "preserveConstEnums": true,                       /* Disable erasing 'const enum' declarations in generated code. */
+    // "declarationDir": "./",                           /* Specify the output directory for generated declaration files. */
+    // "preserveValueImports": true,                     /* Preserve unused imported values in the JavaScript output that would otherwise be removed. */
+
+    /* Interop Constraints */
+    // "isolatedModules": true,                          /* Ensure that each file can be safely transpiled without relying on other imports. */
+    // "allowSyntheticDefaultImports": true,             /* Allow 'import x from y' when a module doesn't have a default export. */
+    "esModuleInterop": true,                             /* Emit additional JavaScript to ease support for importing CommonJS modules. This enables 'allowSyntheticDefaultImports' for type compatibility. */
+    // "preserveSymlinks": true,                         /* Disable resolving symlinks to their realpath. This correlates to the same flag in node. */
+    "forceConsistentCasingInFileNames": true,            /* Ensure that casing is correct in imports. */
+
+    /* Type Checking */
+    "strict": true,                                      /* Enable all strict type-checking options. */
+    // "noImplicitAny": true,                            /* Enable error reporting for expressions and declarations with an implied 'any' type. */
+    // "strictNullChecks": true,                         /* When type checking, take into account 'null' and 'undefined'. */
+    // "strictFunctionTypes": true,                      /* When assigning functions, check to ensure parameters and the return values are subtype-compatible. */
+    // "strictBindCallApply": true,                      /* Check that the arguments for 'bind', 'call', and 'apply' methods match the original function. */
+    // "strictPropertyInitialization": true,             /* Check for class properties that are declared but not set in the constructor. */
+    // "noImplicitThis": true,                           /* Enable error reporting when 'this' is given the type 'any'. */
+    // "useUnknownInCatchVariables": true,               /* Default catch clause variables as 'unknown' instead of 'any'. */
+    // "alwaysStrict": true,                             /* Ensure 'use strict' is always emitted. */
+    // "noUnusedLocals": true,                           /* Enable error reporting when local variables aren't read. */
+    // "noUnusedParameters": true,                       /* Raise an error when a function parameter isn't read. */
+    // "exactOptionalPropertyTypes": true,               /* Interpret optional property types as written, rather than adding 'undefined'. */
+    // "noImplicitReturns": true,                        /* Enable error reporting for codepaths that do not explicitly return in a function. */
+    // "noFallthroughCasesInSwitch": true,               /* Enable error reporting for fallthrough cases in switch statements. */
+    // "noUncheckedIndexedAccess": true,                 /* Add 'undefined' to a type when accessed using an index. */
+    // "noImplicitOverride": true,                       /* Ensure overriding members in derived classes are marked with an override modifier. */
+    // "noPropertyAccessFromIndexSignature": true,       /* Enforces using indexed accessors for keys declared using an indexed type. */
+    // "allowUnusedLabels": true,                        /* Disable error reporting for unused labels. */
+    // "allowUnreachableCode": true,                     /* Disable error reporting for unreachable code. */
+
+    /* Completeness */
+    // "skipDefaultLibCheck": true,                      /* Skip type checking .d.ts files that are included with TypeScript. */
+    "skipLibCheck": true                                 /* Skip type checking all .d.ts files. */
+  }
+}


I'm not sure I want to see every option in a list with most commented out. But since we need to get this merged, I'll call this non-blocking for now. Easy to clean up later if I become passionate about it

bfeister · 2023-04-17T21:25:07Z

packages/pwa-kit-react-sdk/.eslintrc.js

+ */
+
+module.exports = {
+    extends: [require.resolve('pwa-kit-dev/configs/eslint')]


bfeister · 2023-04-17T21:27:08Z

packages/pwa-kit-react-sdk/scripts/file-utils.test.js

@@ -4,6 +4,7 @@
 * SPDX-License-Identifier: BSD-3-Clause
 * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
 */
+/* eslint-disable @typescript-eslint/no-var-requires */


Would be preferable to disable this by directory, (e.g. */scripts/*) and filename convention eslint.config.js I thought that was possible with eslint?

bfeister

I don't see any blockers. Would be good to have more time with this, but I'd prefer to merge it, attempt to fix any fallout and then revert if it blows up, rather than back off, re-attempt later and have someone else have to ramp up on all of this context

kevinxh

Not the easiest PR to review 😅 ... I didn't go through every line of code but this change seems safe to me.

* v3: [Spike]Replace watch with nodemon (#1146) Fix Page Designer ImageWithText Link component (#1092) (#1148) Upgrade deprecated dependencies (#1124) Restart login flow if refresh_token is invalid (#1135) Move some util to site-utils to avoid circular imports (#1133) Restore old file name. (#1140) Update eslint configuration (#1129) CI: clarify the environment variables (#1127) Remove react-query-devtools from production build (#1121) Update lerna.json (#1118) Parallelize lighthouse ci (#1126) Increase test timeouts only on CI env (#1123) Remove unused `request` deprecated dependency. (#1125) Upgrade msw to latest (#1100) Add mergeBasket hook (#1114) [V3][Hooks Integration 🪝] Manually update cache for ShopperCustomer (#1113) dont use callback on mutateAsync (#1119) 2-spaces not 4-spaces (#1117) # Conflicts: # packages/internal-lib-build/package-lock.json # packages/pwa-kit-create-app/package-lock.json # packages/pwa-kit-dev/package-lock.json # packages/pwa-kit-dev/package.json # packages/pwa-kit-dev/src/configs/webpack/config.js # packages/pwa-kit-dev/src/ssr/server/build-dev-server.js # packages/pwa-kit-react-sdk/package-lock.json # packages/pwa-kit-runtime/package-lock.json # packages/template-express-minimal/package-lock.json # packages/template-mrt-reference-app/package-lock.json # packages/template-retail-react-app/package-lock.json # packages/template-typescript-minimal/package-lock.json # packages/test-commerce-sdk-react/package-lock.json

…-rehaul * feature/template-extensibility: [Spike]Replace watch with nodemon (#1146) Fix Page Designer ImageWithText Link component (#1092) (#1148) Upgrade deprecated dependencies (#1124) Restart login flow if refresh_token is invalid (#1135) Move some util to site-utils to avoid circular imports (#1133) Restore old file name. (#1140) Update eslint configuration (#1129) CI: clarify the environment variables (#1127) Remove react-query-devtools from production build (#1121) Update lerna.json (#1118) Parallelize lighthouse ci (#1126) Increase test timeouts only on CI env (#1123) Remove unused `request` deprecated dependency. (#1125) Upgrade msw to latest (#1100) Add mergeBasket hook (#1114) [V3][Hooks Integration 🪝] Manually update cache for ShopperCustomer (#1113) dont use callback on mutateAsync (#1119) 2-spaces not 4-spaces (#1117) # Conflicts: # packages/internal-lib-build/package-lock.json # packages/pwa-kit-dev/package-lock.json # packages/pwa-kit-react-sdk/package-lock.json # packages/pwa-kit-runtime/package-lock.json # packages/pwa-kit-runtime/package.json

wjhsf added 30 commits March 29, 2023 11:18

Snip internal deps for easier dep management.

73efdc8

Temporarily disable build script.

9df2da2

Use pwa-kit-dev for linting instead of internal-lib-build.

b0e912d

Update eslint config to support modern TypeScript.

b7757b4

Add eslint-plugin-react-hooks.

705fb4f

Move eslint-plugin-header to monorepo root.

297f21e

The header is only enforced inside the monorepo.

Remove unused eslint-disable directives.

e2d195c

Convert files to modules.

0a21099

Replace react-hooks/exhaustive-deps with use-effect-no-deps

62fae6a

Linter fixes in commerce-sdk-react.

5e8d095

Disable rules about the any type.

d36ceb9

Revert "Convert files to modules."

f99d934

This reverts commit 0a21099.

Ignore no-var-requires.

4d41b35

Remove unused disable directives.

b6f5799

Ignore no-var-requires.

46fc578

Merge branch 'v3' into wjh/update-eslint

c135c8e

Add comment.

d2af6b0

Fix lint errors in commerce-sdk-react.

2e9c7c8

Use pwa-kit-dev to lint pwa-kit-dev.

fa2351a

Fix lint errors in pwa-kit-dev.

069c0f7

Fix lint errors in pwa-kit-react-sdk.

29ff436

Fix lint errors in pwa-kit-runtime.

b0db361

Fix lint errors in template-express-minimal.

cd43682

Fix lint errors in template-mrt-reference-app.

4e9aac8

Fix lint errors in template-retail-react-app.

b5dc106

Fix lint errors in template-typescript-minimal.

4ff954d

Add TypeScript dep to TypeScript template.

0998e87

Add typecheck npm script.

1148379

Fix type errors.

37c38e0

Fix type errors.

0e5b55d

wjhsf added 3 commits April 14, 2023 13:24

Remove unused dependencies.

498631e

Apply new lint rules to root scripts.

548e3ab

Use template-retail-react-app as source of truth, rather than interna…

8c919c5

…l-lib-build.

wjhsf commented Apr 14, 2023

View reviewed changes

wjhsf added 15 commits April 14, 2023 14:04

Remove unnecessary newlines.

7133d01

More small lint fixes.

f9ba676

Fix type errors.

f71d011

Update lockfiles.

491a4ce

A space.

8c1c02f

Add new endpoints with TODO.

bbee379

Revert "Update lockfiles."

1d14606

This reverts commit 491a4ce.

Revert "Add new endpoints with TODO."

e189229

This reverts commit bbee379.

Rename file.

80e3824

Split config into partials to facilitate creating alternate configura…

b170bda

…tions.

Add lint config that doesn't use React.

6c0f21a

Add "safe types" eslint config.

657d491

Update eslint configs to point to new files.

20d8388

Remove hard-coded config.

df2661d

Add README for lint configs.

139e663

wjhsf commented Apr 17, 2023

View reviewed changes

bfeister reviewed Apr 17, 2023

View reviewed changes

packages/pwa-kit-react-sdk/.eslintrc.js

*/

module.exports = {

extends: [require.resolve('pwa-kit-dev/configs/eslint')]

Copy link

Collaborator

bfeister Apr 17, 2023

Choose a reason for hiding this comment

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

🎉

bfeister reviewed Apr 17, 2023

View reviewed changes

bfeister approved these changes Apr 17, 2023

View reviewed changes

kevinxh approved these changes Apr 17, 2023

View reviewed changes

wjhsf merged commit 0437c10 into v3 Apr 18, 2023

wjhsf deleted the wjh/update-eslint branch April 18, 2023 13:38

wjhsf mentioned this pull request Apr 18, 2023

Restore eslint-config.js file for backwards compatibility #1140

Merged

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update eslint configuration #1129

Update eslint configuration #1129

wjhsf commented Apr 13, 2023 •

edited

Loading

wjhsf Apr 14, 2023

wjhsf Apr 17, 2023

wjhsf Apr 17, 2023

wjhsf Apr 17, 2023

bfeister Apr 17, 2023

bfeister Apr 17, 2023

bfeister Apr 17, 2023

bfeister left a comment

kevinxh left a comment

Update eslint configuration #1129

Update eslint configuration #1129

Conversation

wjhsf commented Apr 13, 2023 • edited Loading

Description

Types of Changes

Changes

How to Test-Drive This PR

Checklists

General

Accessibility Compliance

Localization

wjhsf Apr 14, 2023

Choose a reason for hiding this comment

wjhsf Apr 17, 2023

Choose a reason for hiding this comment

wjhsf Apr 17, 2023

Choose a reason for hiding this comment

wjhsf Apr 17, 2023

Choose a reason for hiding this comment

bfeister Apr 17, 2023

Choose a reason for hiding this comment

bfeister Apr 17, 2023

Choose a reason for hiding this comment

bfeister Apr 17, 2023

Choose a reason for hiding this comment

bfeister left a comment

Choose a reason for hiding this comment

kevinxh left a comment

Choose a reason for hiding this comment

wjhsf commented Apr 13, 2023 •

edited

Loading