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.

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

Typescript 4.5 upgrade, loads of new linting #1351

Merged
merged 30 commits into from Dec 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
2382ac6
TypeScript 4.5 and Prettier 2.5 (for TS 4.5 syntax support
Stephen-ONeil Dec 22, 2021
41e6317
Drop deprectaed prettier rule jsxBracketSameLine
Stephen-ONeil Dec 22, 2021
405187d
Update eslint from 7 to 8, corresponding versions of @typescript-esli…
Stephen-ONeil Dec 22, 2021
ddfc0d2
Rename root package.json prettier scripts, add eslint scripts
Stephen-ONeil Dec 22, 2021
2675226
Add a prettier write call post eslint:fix, closes #1213
Stephen-ONeil Dec 22, 2021
8bbcf63
Change prettier:write log level to warn, default logging is overly ve…
Stephen-ONeil Dec 22, 2021
ad8eb07
Use consistent, desired, configuration for no-unused-vars in both js …
Stephen-ONeil Dec 22, 2021
682d119
Centralize eslint ignore rules at top level, like we have with the pr…
Stephen-ONeil Dec 22, 2021
ac948b8
Misc linting
Stephen-ONeil Dec 22, 2021
8bac211
Sort out linting in root scripts, fix some lint errors there and in f…
Stephen-ONeil Dec 22, 2021
39a8d39
Lint instance of no-use-before-define in server code
Stephen-ONeil Dec 22, 2021
c8400b7
Add stand alone repo-wide lint job to CI
Stephen-ONeil Dec 22, 2021
673c70e
Configure repo wide eslint command to cache
Stephen-ONeil Dec 22, 2021
85f922d
Make use of eslint caches in CI lint job
Stephen-ONeil Dec 22, 2021
93cefc3
Lint no-unused-vars in form_backend and server
Stephen-ONeil Dec 22, 2021
eb0261f
Extract CI install steps to fragments, to make sure cache keys are co…
Stephen-ONeil Dec 22, 2021
5fcd131
Ah, CircleCI has a "commands" syntax for reusable blocks of steps, th…
Stephen-ONeil Dec 22, 2021
8fb128b
Set max warnings to 0 for repo wide eslint scan, mainly so CI gets th…
Stephen-ONeil Dec 22, 2021
6ca94c3
Fix rest arg error introduced by removing an unused var in the server…
Stephen-ONeil Dec 22, 2021
7605694
Set ignoreRestSiblings: true in no-unused-vars lint rule, a bit of a …
Stephen-ONeil Dec 22, 2021
f73ee5d
Add TODO on CI lint job, consider expanding to check prettier and typ…
Stephen-ONeil Dec 22, 2021
ec22b92
Tangent, drop some old stories modules I missed
Stephen-ONeil Dec 22, 2021
52a65b8
Lint for no-unused vars in client (everywhere outside of panels and c…
Stephen-ONeil Dec 22, 2021
9b851dc
Lint the rest of the client no-unused-vars
Stephen-ONeil Dec 22, 2021
ab836e5
Enable @typescript-eslint/consistent-type-imports, auto-fix it
Stephen-ONeil Dec 22, 2021
1e567ce
Post-rebase auto fix some consistent-type-imports instances
Stephen-ONeil Dec 22, 2021
b642f09
Add prettier and typescript checking alongside eslint in CI static_an…
Stephen-ONeil Dec 22, 2021
9dd236c
Drop prettier from pre-push hook, CI's responsibility now
Stephen-ONeil Dec 22, 2021
8732ea2
Skip lint and TS checking during CI builds, since they happen in a se…
Stephen-ONeil Dec 22, 2021
20d17fb
Drop not-CI conditional on build time lint and TS checks, not a subst…
Stephen-ONeil Dec 22, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
146 changes: 91 additions & 55 deletions .circleci/config.yml
@@ -1,44 +1,112 @@
version: 2.1

dockerhub_auth: &dockerhub_auth
auth:
username: $DOCKERHUB_USERNAME
password: $DOCKERHUB_PASSWORD

# yaml fragment for deploy filters
deploy_filter: &deploy_filter
filters:
branches:
ignore:
- /__.*/

version: 2.1

jobs:
test_form_backend:
docker:
- image: eaadtbs/ib-ci-build:3.0
<<: *dockerhub_auth
working_directory: "~/InfoBase"
commands:
top_level_install:
steps:
- checkout:
path: "~/InfoBase"
- restore_cache:
keys:
- form-backend-dependencies-{{ checksum "form_backend/package-lock.json" }}-v2
- top-level-dependencies-{{ checksum "package-lock.json" }}-v2
- run:
command: |
[ -e "form_backend/node_modules" ] || (cd form_backend && npm ci)
[ -e "node_modules" ] || npm ci
- save_cache:
key: top-level-dependencies-{{ checksum "package-lock.json" }}-v2
paths:
- node_modules
form_backend_install:
steps:
- restore_cache:
keys:
- form-backend-dependencies-{{ checksum "form_backend/package-lock.json" }}-v2
- run:
command: |
[ -e "node_modules" ] || npm ci
[ -e "form_backend/node_modules" ] || (cd form_backend && npm ci)
- save_cache:
key: form-backend-dependencies-{{ checksum "form_backend/package-lock.json" }}-v2
paths:
- form_backend/node_modules
server_install:
steps:
- restore_cache:
keys:
- server-dependencies-{{ checksum "server/package-lock.json" }}-v2
- run:
command: |
[ -e "server/node_modules" ] || (cd server && npm ci)
- save_cache:
key: top-level-dependencies-{{ checksum "package-lock.json" }}-v2
key: server-dependencies-{{ checksum "server/package-lock.json" }}-v2
paths:
- node_modules
- server/node_modules
client_install:
steps:
# need to bust the client package cache on patch changes in addition to package-lock changes
- run: cksum ./client/patches/* > client-patch-checksums
- restore_cache:
keys:
- client-dependencies-{{ checksum "client/package-lock.json" }}-{{ checksum "client-patch-checksums" }}-v2
- run:
command: |
[ -e "client/node_modules" ] || (cd client && npm ci)
- save_cache:
key: client-dependencies-{{ checksum "client/package-lock.json" }}-{{ checksum "client-patch-checksums" }}-v2
paths:
- client/node_modules
- client/.cache/Cypress # path set by CYPRESS_CACHE_FOLDER env var

jobs:
static_analysis:
docker:
- image: eaadtbs/ib-ci-build:3.0
<<: *dockerhub_auth
working_directory: "~/InfoBase"
steps:
- checkout:
path: "~/InfoBase"

- top_level_install
- form_backend_install
- server_install
- client_install

- restore_cache:
keys:
- repo-wide-lint-cache-{{ .Branch }}
- repo-wide-lint-cache-master
- run:
command: npm run eslint
- save_cache:
key: repo-wide-lint-cache-{{ .Branch }}
paths:
- .eslintcache

- run:
command: npm run prettier

- run:
command: cd client && npx tsc --noEmit

test_form_backend:
docker:
- image: eaadtbs/ib-ci-build:3.0
<<: *dockerhub_auth
working_directory: "~/InfoBase"
steps:
- checkout:
path: "~/InfoBase"

- top_level_install
- form_backend_install

- restore_cache:
keys:
Expand Down Expand Up @@ -90,49 +158,15 @@ jobs:
- image: eaadtbs/ib-ci-build:3.0
<<: *dockerhub_auth
working_directory: "~/InfoBase"
environment:
CYPRESS_CACHE_FOLDER: "~/InfoBase/client/.cache/Cypress"
resource_class: medium
steps:
- checkout:
path: "~/InfoBase"
- run: ./scripts/ci_scripts/create_envs.sh

- restore_cache:
keys:
- top-level-dependencies-{{ checksum "package-lock.json" }}-v4
- run:
command: |
[ -e "node_modules" ] || npm ci
- save_cache:
key: top-level-dependencies-{{ checksum "package-lock.json" }}-v4
paths:
- node_modules

# need to bust the client package cache on patch changes in addition to package-lock changes
- run: cksum ./client/patches/* > client-patch-checksums
- restore_cache:
keys:
- client-dependencies-{{ checksum "client/package-lock.json" }}-{{ checksum "client-patch-checksums" }}-v2
- run:
command: |
[ -e "client/node_modules" ] || (cd client && npm ci)
- save_cache:
key: client-dependencies-{{ checksum "client/package-lock.json" }}-{{ checksum "client-patch-checksums" }}-v2
paths:
- client/node_modules
- client/.cache/Cypress

- restore_cache:
keys:
- server-dependencies-{{ checksum "server/package-lock.json" }}-v2
- run:
command: |
[ -e "server/node_modules" ] || (cd server && npm ci)
- save_cache:
key: server-dependencies-{{ checksum "server/package-lock.json" }}-v2
paths:
- server/node_modules
- top_level_install
- client_install
- server_install

# transpiling the server slightly out of place for this job, not needed till the server deploy step, but
# the deploy job image isn't guaranteed to have the necessary environment for this
Expand Down Expand Up @@ -272,8 +306,6 @@ jobs:
- image: eaadtbs/ib-ci-cypress:4.0
<<: *dockerhub_auth
working_directory: "~/InfoBase"
environment:
CYPRESS_CACHE_FOLDER: "~/InfoBase/client/.cache/Cypress"
parameters:
batch-count:
type: integer
Expand Down Expand Up @@ -380,6 +412,10 @@ jobs:
- run: if [[ $CIRCLE_BRANCH = master ]]; then ./scripts/ci_scripts/store_bundle_stats.sh; fi

workflows:
static_analysis:
jobs:
- static_analysis

test_form_backend:
jobs:
- test_form_backend
Expand Down
7 changes: 7 additions & 0 deletions .eslintignore
@@ -0,0 +1,7 @@
*/.coverage
*/.cache
*/transpiled_build
client/build
client/build_prod
client/src/**/*.scss.d.ts
client/scripts/typescript_progress/output
5 changes: 4 additions & 1 deletion .eslintrc.json
Expand Up @@ -8,7 +8,10 @@
"plugin:import/errors"
],
"rules": {
"no-unused-vars": ["warn", { "args": "none" }],
"no-unused-vars": [
"warn",
{ "ignoreRestSiblings": true, "argsIgnorePattern": "^_.+" }
],
"no-use-before-define": ["error", { "functions": true, "classes": true }],
"no-throw-literal": "error",
"lodash/no-double-unwrap": ["error"],
Expand Down
3 changes: 2 additions & 1 deletion .gitignore
Expand Up @@ -20,4 +20,5 @@ npm-debug*
node_modules/
.local_mongo
.coverage
.cache
.cache
.eslintcache
6 changes: 1 addition & 5 deletions .husky/pre-push
Expand Up @@ -20,8 +20,4 @@ local_branch_name=$(echo "$local_ref" | sed "s/refs\/heads\///")
if [[ ($local_branch_name =~ ^__ || $local_branch_name =~ ^archived__ ) && $remote != "archive" ]]; then
echo -e "\033[0;31mDouble underscore and archived branches can only be pushed to the archive (private) remote!\033[0m"
exit 1
fi


# run prettier check
npm run prettier_check .
fi
3 changes: 0 additions & 3 deletions client/.eslintignore

This file was deleted.

10 changes: 7 additions & 3 deletions client/.eslintrc.json
Expand Up @@ -47,8 +47,11 @@
],
"rules": {
"no-prototype-builtins": "off", //common TS pattern
"@typescript-eslint/no-unused-vars": "off", // Lot of false positives on this rule. Better to use the exprimental
"@typescript-eslint/no-unused-vars-experimental": "warn",
"no-unused-vars": "off",
"@typescript-eslint/no-unused-vars": [
"warn",
{ "ignoreRestSiblings": true, "argsIgnorePattern": "^_.+" }
],
"@typescript-eslint/no-redeclare": ["error"],
"@typescript-eslint/explicit-module-boundary-types": "off", // TODO might want to turn this on later, but it will take a while to fix all the warnings
"@typescript-eslint/ban-types": [
Expand All @@ -66,7 +69,8 @@
}
}
}
]
],
"@typescript-eslint/consistent-type-imports": "error"
}
}
],
Expand Down
2 changes: 1 addition & 1 deletion client/build_code/webpack_common.js
Expand Up @@ -145,7 +145,7 @@ function get_plugins({
}),
new CircularDependencyPlugin({
exclude: /node_modules/,
onDetected({ module: webpackModuleRecord, paths, compilation }) {
onDetected({ paths, compilation }) {
/*
Reminder: circular dependencies aren't _necessarily_ problematic. The concern is that when they _are_ the source of a bug,
they can be a very hard to identify source. Best to avoid creating them, but at a certain point avoiding them can start
Expand Down
4 changes: 2 additions & 2 deletions client/build_code/write_footnote_bundles.js
Expand Up @@ -36,7 +36,7 @@ function populate_stores(parsed_models) {
crso_deptcodes[id] = dept_code;
});

_.each(programs, ({ dept_code, activity_code, crso_id }) => {
_.each(programs, ({ dept_code, activity_code }) => {
const prog_id = `${dept_code}-${activity_code}`;
program_deptcodes[prog_id] = dept_code;
});
Expand All @@ -50,7 +50,7 @@ function populate_stores(parsed_models) {

//initialize all depts and tags to have empty array of footnotes
footnotes_by_deptcode = _.chain(depts)
.map(({ org_id, dept_code }) => [dept_code, []])
.map(({ dept_code }) => [dept_code, []])
.fromPairs()
.value();

Expand Down
2 changes: 1 addition & 1 deletion client/cypress/integration/InfoBase/route_tests.spec.js
Expand Up @@ -322,7 +322,7 @@ const run_tests_from_config = ({
if (expect_error_boundary) {
// To catch whatever error triggered this error boundary, otherwise this would fail even if it
// results in the expected error boundary
cy.on("fail", (e, test) => {
cy.on("fail", (e) => {
console.log(
"This test was expected to fail. It did, with the following error",
e
Expand Down
2 changes: 1 addition & 1 deletion client/cypress/plugins/index.js
Expand Up @@ -3,7 +3,7 @@
*/

// eslint-disable-next-line import/no-commonjs
module.exports = (on, config) => {
module.exports = (on) => {
on("task", {
log(message) {
console.log(message);
Expand Down