From 0265cf70bd39b9a38a51184b731bf3ec38e14a88 Mon Sep 17 00:00:00 2001 From: Paul Berberian Date: Wed, 4 Sep 2024 11:30:06 +0200 Subject: [PATCH 1/7] Run eslint on the scripts directory I noticed that the `scripts` directory, which begins to have a lot of files as we automate more tasks (like #1524), wasn't linted. That directory contains code in two languages BASH and JS (Node.js), with a move to prefer JS files (as it's a JS/TS project). So I chose to add an `.eslintrc.js` file (which is a deprecated format but I couldn't make the new one work) and lint script on that directory. I subsequently fixed some linting errors. For now, that script isn't run by our GitHub actions, I will see whether we can do an intelligent kind of runner which detects if scripts have been updated. --- package.json | 3 ++- scripts/.eslintrc.js | 53 +++++++++++++++++++++++++++++++++++++++++ scripts/run_bundler.mjs | 6 +++-- 3 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 scripts/.eslintrc.js diff --git a/package.json b/package.json index ba66acf7cd..8e58a53784 100644 --- a/package.json +++ b/package.json @@ -151,7 +151,7 @@ "bundle:watch": "./scripts/run_bundler.mjs src/index.ts --production-mode -o dist/rx-player.js --watch", "certificate": "./scripts/generate_certificate", "check": "npm run check:types && npm run lint && npm run check:types:unit_tests", - "check:all": "npm run check:types && npm run lint && npm run lint:demo && npm run lint:tests && npm run test:unit && npm run test:integration && npm run test:memory && node -r esm ./scripts/check_nodejs_import_compatibility.js", + "check:all": "npm run check:types && npm run lint && npm run lint:demo && npm run lint:tests && npm run lint:scripts && npm run test:unit && npm run test:integration && npm run test:memory && node -r esm ./scripts/check_nodejs_import_compatibility.js", "check:demo": "npm run check:demo:types && npm run lint:demo", "check:demo:types": "tsc --noEmit --project demo/", "clean:build": "scripts/remove_dir.mjs dist", @@ -169,6 +169,7 @@ "fmt:rust:check": "cd ./src/parsers/manifest/dash/wasm-parser && cargo fmt --check", "lint": "eslint src -c .eslintrc.js", "lint:demo": "eslint -c demo/.eslintrc.js demo/scripts", + "lint:scripts": "eslint -c scripts/.eslintrc.js --ext .js --ext .mjs --ext .cjs scripts", "lint:tests": "eslint tests/**/*.js --ignore-pattern '/tests/performance/bundle*'", "list": "node scripts/list-npm-scripts.mjs", "prepublishOnly": "npm run build:all", diff --git a/scripts/.eslintrc.js b/scripts/.eslintrc.js new file mode 100644 index 0000000000..b1482a8a0a --- /dev/null +++ b/scripts/.eslintrc.js @@ -0,0 +1,53 @@ +module.exports = { + root: true, + parserOptions: { + ecmaVersion: 2020, + sourceType: "module", + }, + env: { + es6: true, + node: true, + commonjs: true, + }, + extends: ["prettier"], + rules: { + "comma-dangle": [1, "only-multiline"], + "no-cond-assign": 0, + "no-constant-condition": 0, + "no-control-regex": 0, + "no-debugger": 2, + "no-dupe-args": 2, + "no-dupe-keys": 2, + "no-duplicate-case": 1, + "no-empty-character-class": 1, + "no-empty": 0, + "no-ex-assign": 1, + "no-extra-boolean-cast": 1, + "no-extra-parens": [1, "functions"], + "no-extra-semi": 2, + "no-func-assign": 2, + "no-inner-declarations": 0, + "no-invalid-regexp": 1, + "no-irregular-whitespace": 1, + "no-negated-in-lhs": 2, + "no-obj-calls": 2, + "no-regex-spaces": 1, + "no-sparse-arrays": 2, + "no-unreachable": 2, + "use-isnan": 2, + "valid-jsdoc": 0, + "valid-typeof": 2, + "no-unexpected-multiline": 0, + "no-trailing-spaces": 2, + "no-multiple-empty-lines": 1, + "accessor-pairs": [1, { setWithoutGet: true }], + "block-scoped-var": 1, + complexity: 0, + curly: [1, "all"], + "no-case-declarations": 0, + "no-var": 1, + "prefer-const": 1, + "linebreak-style": [1, "unix"], + semi: [1, "always"], + }, +}; diff --git a/scripts/run_bundler.mjs b/scripts/run_bundler.mjs index 43456733a3..ce1f0718bf 100755 --- a/scripts/run_bundler.mjs +++ b/scripts/run_bundler.mjs @@ -62,16 +62,18 @@ if (import.meta.url === pathToFileURL(process.argv[1]).href) { } try { - await runBundler(normalizedPath, { + runBundler(normalizedPath, { watch: shouldWatch, minify: shouldMinify, production, silent, outfile, + }).catch((err) => { + console.error(`ERROR: ${err}\n`); + process.exit(1); }); } catch (err) { console.error(`ERROR: ${err}\n`); - displayHelp(); process.exit(1); } } From 2e3173fbad17cecf7e2bc11b7f1d0f39d57b1dd0 Mon Sep 17 00:00:00 2001 From: Paul Berberian Date: Wed, 4 Sep 2024 11:34:55 +0200 Subject: [PATCH 2/7] Add lint script check --- .github/workflows/script_check.yml | 31 ++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 .github/workflows/script_check.yml diff --git a/.github/workflows/script_check.yml b/.github/workflows/script_check.yml new file mode 100644 index 0000000000..e001fc6d5d --- /dev/null +++ b/.github/workflows/script_check.yml @@ -0,0 +1,31 @@ +# This workflow will do a clean install of node dependencies, cache/restore them, build the source code and run tests across different versions of node +# For more information see: https://help.github.com/actions/language-and-framework-guides/using-nodejs-with-github-actions + +name: RxPlayer Script Checks + +on: + push: + branches: [stable, dev, legacy-v3] + paths: + - scripts/** + pull_request: + types: [opened, synchronize, reopened] + +jobs: + scripts_lint: + runs-on: ubuntu-latest + + strategy: + matrix: + node-version: [20.x] + # See supported Node.js release schedule at https://nodejs.org/en/about/releases/ + + steps: + - uses: actions/checkout@v2 + - name: Use Node.js ${{ matrix.node-version }} + uses: actions/setup-node@v2 + with: + node-version: ${{ matrix.node-version }} + cache: "npm" + - run: npm install + - run: npm run lint:scripts From 93fc95fc408c43dbe290b4c15cb687a33a18924f Mon Sep 17 00:00:00 2001 From: Paul Berberian Date: Wed, 4 Sep 2024 11:37:02 +0200 Subject: [PATCH 3/7] Add demo check to github actions --- .github/workflows/demo_check.yml | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 .github/workflows/demo_check.yml diff --git a/.github/workflows/demo_check.yml b/.github/workflows/demo_check.yml new file mode 100644 index 0000000000..b69fee909f --- /dev/null +++ b/.github/workflows/demo_check.yml @@ -0,0 +1,31 @@ +# This workflow will do a clean install of node dependencies, cache/restore them, build the source code and run tests across different versions of node +# For more information see: https://help.github.com/actions/language-and-framework-guides/using-nodejs-with-github-actions + +name: RxPlayer Demo Checks + +on: + push: + branches: [stable, dev, legacy-v3] + paths: + - demo/** + pull_request: + types: [opened, synchronize, reopened] + +jobs: + scripts_lint: + runs-on: ubuntu-latest + + strategy: + matrix: + node-version: [20.x] + # See supported Node.js release schedule at https://nodejs.org/en/about/releases/ + + steps: + - uses: actions/checkout@v2 + - name: Use Node.js ${{ matrix.node-version }} + uses: actions/setup-node@v2 + with: + node-version: ${{ matrix.node-version }} + cache: "npm" + - run: npm install + - run: npm run check:demo From f68bccbb30760ee92149637e884a1b485193b5a9 Mon Sep 17 00:00:00 2001 From: Paul Berberian Date: Wed, 4 Sep 2024 11:38:06 +0200 Subject: [PATCH 4/7] Remove unnecessary top notice in github actions files --- .github/workflows/checks.yml | 3 --- .github/workflows/demo_check.yml | 3 --- .github/workflows/script_check.yml | 3 --- 3 files changed, 9 deletions(-) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 859bf2b5ff..a4affa57fe 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -1,6 +1,3 @@ -# This workflow will do a clean install of node dependencies, cache/restore them, build the source code and run tests across different versions of node -# For more information see: https://help.github.com/actions/language-and-framework-guides/using-nodejs-with-github-actions - name: RxPlayer Tests on: diff --git a/.github/workflows/demo_check.yml b/.github/workflows/demo_check.yml index b69fee909f..b876723abf 100644 --- a/.github/workflows/demo_check.yml +++ b/.github/workflows/demo_check.yml @@ -1,6 +1,3 @@ -# This workflow will do a clean install of node dependencies, cache/restore them, build the source code and run tests across different versions of node -# For more information see: https://help.github.com/actions/language-and-framework-guides/using-nodejs-with-github-actions - name: RxPlayer Demo Checks on: diff --git a/.github/workflows/script_check.yml b/.github/workflows/script_check.yml index e001fc6d5d..c6183191c0 100644 --- a/.github/workflows/script_check.yml +++ b/.github/workflows/script_check.yml @@ -1,6 +1,3 @@ -# This workflow will do a clean install of node dependencies, cache/restore them, build the source code and run tests across different versions of node -# For more information see: https://help.github.com/actions/language-and-framework-guides/using-nodejs-with-github-actions - name: RxPlayer Script Checks on: From cb034f932f259aeee2e2475664c4de0bd4ab20ea Mon Sep 17 00:00:00 2001 From: Paul Berberian Date: Wed, 4 Sep 2024 11:39:25 +0200 Subject: [PATCH 5/7] Fix github actions conditions --- .github/workflows/demo_check.yml | 6 ++---- .github/workflows/script_check.yml | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/.github/workflows/demo_check.yml b/.github/workflows/demo_check.yml index b876723abf..c17a18fe4a 100644 --- a/.github/workflows/demo_check.yml +++ b/.github/workflows/demo_check.yml @@ -1,12 +1,10 @@ name: RxPlayer Demo Checks on: - push: - branches: [stable, dev, legacy-v3] - paths: - - demo/** pull_request: types: [opened, synchronize, reopened] + paths: + - demo/** jobs: scripts_lint: diff --git a/.github/workflows/script_check.yml b/.github/workflows/script_check.yml index c6183191c0..24a3523986 100644 --- a/.github/workflows/script_check.yml +++ b/.github/workflows/script_check.yml @@ -1,12 +1,10 @@ name: RxPlayer Script Checks on: - push: - branches: [stable, dev, legacy-v3] - paths: - - scripts/** pull_request: types: [opened, synchronize, reopened] + paths: + - scripts/** jobs: scripts_lint: From 15bcb3dcdad4120690bcac498119020c69577b61 Mon Sep 17 00:00:00 2001 From: Paul Berberian Date: Wed, 4 Sep 2024 11:44:32 +0200 Subject: [PATCH 6/7] Add test linting github action --- .github/workflows/tests_check.yml | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 .github/workflows/tests_check.yml diff --git a/.github/workflows/tests_check.yml b/.github/workflows/tests_check.yml new file mode 100644 index 0000000000..9daf64d05f --- /dev/null +++ b/.github/workflows/tests_check.yml @@ -0,0 +1,26 @@ +name: RxPlayer Tests Checks + +on: + pull_request: + types: [opened, synchronize, reopened] + paths: + - tests/** + +jobs: + tests_lint: + runs-on: ubuntu-latest + + strategy: + matrix: + node-version: [20.x] + # See supported Node.js release schedule at https://nodejs.org/en/about/releases/ + + steps: + - uses: actions/checkout@v2 + - name: Use Node.js ${{ matrix.node-version }} + uses: actions/setup-node@v2 + with: + node-version: ${{ matrix.node-version }} + cache: "npm" + - run: npm install + - run: npm run lint:tests From 727b886687d24159cc5bfa8a81ec5b3e7e31b762 Mon Sep 17 00:00:00 2001 From: Paul Berberian Date: Tue, 24 Sep 2024 17:50:40 +0200 Subject: [PATCH 7/7] CI: use npm ci instead of install everywhere --- .github/workflows/checks.yml | 10 +++++----- .github/workflows/demo_check.yml | 2 +- .github/workflows/perfs.yml | 2 +- .github/workflows/script_check.yml | 2 +- .github/workflows/tests_check.yml | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index a4affa57fe..3bc146b50b 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -26,7 +26,7 @@ jobs: with: toolchain: stable - - run: npm install + - run: npm ci - run: rustup target add wasm32-unknown-unknown - run: npm run fmt:prettier:check - run: npm run fmt:rust:check @@ -46,7 +46,7 @@ jobs: with: node-version: ${{ matrix.node-version }} cache: "npm" - - run: npm install + - run: npm ci - run: npm run check unit_tests: @@ -64,7 +64,7 @@ jobs: with: node-version: ${{ matrix.node-version }} cache: "npm" - - run: npm install + - run: npm ci - run: npm run test:unit integration_linux: @@ -86,7 +86,7 @@ jobs: - run: sudo add-apt-repository multiverse && sudo apt update && sudo apt install -y ubuntu-restricted-extras - - run: npm install + - run: npm ci - run: npm run build - run: npm run test:integration @@ -109,5 +109,5 @@ jobs: - run: sudo add-apt-repository multiverse && sudo apt update && sudo apt install -y ubuntu-restricted-extras - - run: npm install + - run: npm ci - run: npm run test:memory diff --git a/.github/workflows/demo_check.yml b/.github/workflows/demo_check.yml index c17a18fe4a..14c8790396 100644 --- a/.github/workflows/demo_check.yml +++ b/.github/workflows/demo_check.yml @@ -22,5 +22,5 @@ jobs: with: node-version: ${{ matrix.node-version }} cache: "npm" - - run: npm install + - run: npm ci - run: npm run check:demo diff --git a/.github/workflows/perfs.yml b/.github/workflows/perfs.yml index 83c8853b21..326f916813 100644 --- a/.github/workflows/perfs.yml +++ b/.github/workflows/perfs.yml @@ -18,7 +18,7 @@ jobs: - run: sudo add-apt-repository multiverse && sudo apt update && sudo apt install -y ubuntu-restricted-extras - - run: npm install + - run: npm ci - run: export DISPLAY=:99 - run: sudo Xvfb -ac :99 -screen 0 1280x1024x24 > /dev/null 2>&1 & # optional - run: node tests/performance/run.mjs diff --git a/.github/workflows/script_check.yml b/.github/workflows/script_check.yml index 24a3523986..15f9f36101 100644 --- a/.github/workflows/script_check.yml +++ b/.github/workflows/script_check.yml @@ -22,5 +22,5 @@ jobs: with: node-version: ${{ matrix.node-version }} cache: "npm" - - run: npm install + - run: npm ci - run: npm run lint:scripts diff --git a/.github/workflows/tests_check.yml b/.github/workflows/tests_check.yml index 9daf64d05f..50165c3ba2 100644 --- a/.github/workflows/tests_check.yml +++ b/.github/workflows/tests_check.yml @@ -22,5 +22,5 @@ jobs: with: node-version: ${{ matrix.node-version }} cache: "npm" - - run: npm install + - run: npm ci - run: npm run lint:tests