Skip to content

Commit 8ea1cff

Browse files
authored
chore: add ESLint linting to the project (#84)
Set up ESLint with @eslint/js, @stylistic/eslint-plugin, eslint-plugin-n, eslint-plugin-unicorn, and eslint-plugin-import-x. Add an eslint.config.js with project-specific overrides (CJS mode, Node.js >=18, 1tbs brace style, kebab-case filenames, prefer-const, import ordering, etc.) and a lint CI job in the build workflow. Apply all auto-fixable and manual lint fixes across the codebase: - Use node: protocol for built-in requires - Replace var/let with const where appropriate - Add trailing commas, remove semicolons, fix spacing - Rename files to kebab-case (test_utils.js -> test-utils.js, test_wasm.js -> test-wasm.js, process_discovery.js -> process-discovery.js) - Replace .find() equality checks with .includes() - Remove unused catch bindings - Use top-level await where applicable - Auto-install test subdirectory dependencies in test.sh
1 parent 673d854 commit 8ea1cff

23 files changed

+1193
-139
lines changed

.github/actions/build-test-wasm/action.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ runs:
2222
wasm-pack build --target nodejs ./crates/${{ inputs.crate }} --out-dir ../../prebuilds/${{ inputs.crate }}
2323
shell: bash
2424
- name: Test WASM
25-
run: node test_wasm.js ${{ inputs.crate }}
25+
run: node test-wasm.js ${{ inputs.crate }}
2626
shell: bash
2727
- uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
2828
with:

.github/workflows/build.yml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,17 @@ on:
77
- main
88

99
jobs:
10+
lint:
11+
runs-on: ubuntu-latest
12+
steps:
13+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1
14+
- name: Setup Node.js
15+
uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0
16+
- run: yarn install
17+
- run: yarn install
18+
working-directory: test/crashtracker
19+
- run: yarn lint
20+
1021
build-test-wasm:
1122
runs-on: ubuntu-latest
1223
strategy:

.yarnrc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
--ignore-engines true

eslint.config.js

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
'use strict'
2+
3+
const eslintPluginImportX = require('eslint-plugin-import-x')
4+
const eslintPluginJs = require('@eslint/js')
5+
const eslintPluginN = require('eslint-plugin-n')
6+
const eslintPluginStylistic = require('@stylistic/eslint-plugin')
7+
const eslintPluginUnicorn = require('eslint-plugin-unicorn').default
8+
const globals = require('globals')
9+
10+
module.exports = [
11+
eslintPluginJs.configs.recommended,
12+
eslintPluginImportX.flatConfigs.recommended,
13+
eslintPluginN.configs['flat/recommended-script'],
14+
eslintPluginStylistic.configs.recommended,
15+
eslintPluginUnicorn.configs.recommended,
16+
{
17+
languageOptions: {
18+
ecmaVersion: 2022,
19+
sourceType: 'commonjs',
20+
globals: {
21+
...globals.es2022,
22+
...globals.node,
23+
},
24+
},
25+
settings: {
26+
// Used by `eslint-plugin-n` to determine the minimum version of Node.js to support.
27+
// Normally setting this in the `package.json` engines field is enough, but we can't use that as it will fail
28+
// when running `yarn copy-artifacts` inside the prebuildify Docker container which uses Node.js 12.
29+
node: { version: '>=18.0.0' },
30+
},
31+
rules: {
32+
'@stylistic/brace-style': ['error', '1tbs'],
33+
'@stylistic/space-before-function-paren': ['error', 'always'],
34+
'import-x/extensions': ['error', 'never', { json: 'always' }],
35+
'import-x/no-absolute-path': 'error',
36+
'import-x/no-webpack-loader-syntax': 'error',
37+
'import-x/order': ['error', {
38+
'newlines-between': 'always',
39+
}],
40+
'n/no-process-exit': 'off', // Duplicate of unicorn/no-process-exit
41+
'prefer-const': 'error',
42+
'unicorn/prefer-module': 'off', // We use CJS
43+
'unicorn/prevent-abbreviations': 'off',
44+
},
45+
},
46+
{
47+
files: ['load.js'],
48+
languageOptions: {
49+
globals: {
50+
__webpack_require__: 'readonly',
51+
__non_webpack_require__: 'readonly',
52+
},
53+
},
54+
},
55+
{
56+
// This script runs inside the prebuildify Docker container which uses Node.js 12
57+
files: ['scripts/copy-artifacts.js'],
58+
languageOptions: {
59+
ecmaVersion: 2019,
60+
},
61+
settings: {
62+
// Used by `eslint-plugin-n` to determine the minimum version of Node.js to support.
63+
node: { version: '>=12.0.0' },
64+
},
65+
rules: {
66+
'unicorn/prefer-node-protocol': 'off',
67+
},
68+
},
69+
{
70+
ignores: ['build/', 'target/', 'prebuilds/'],
71+
},
72+
]

load.js

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@
22

33
// TODO: Extract this file to an external library.
44

5-
const { existsSync, readdirSync } = require('fs')
6-
const os = require('os')
7-
const path = require('path')
5+
const { existsSync, readdirSync } = require('node:fs')
6+
const os = require('node:os')
7+
const path = require('node:path')
88

99
const PLATFORM = os.platform()
1010
const ARCH = process.arch
11-
const LIBC = PLATFORM === 'linux' ? existsSync('/etc/alpine-release') ? 'musl' : 'glibc' : ''
11+
const LIBC = PLATFORM === 'linux' ? (existsSync('/etc/alpine-release') ? 'musl' : 'glibc') : ''
1212
const ABI = process.versions.modules
1313

1414
const inWebpack = typeof __webpack_require__ === 'function'
@@ -17,7 +17,7 @@ const runtimeRequire = inWebpack ? __non_webpack_require__ : require
1717
function maybeLoad (name) {
1818
try {
1919
return load(name)
20-
} catch (e) {
20+
} catch {
2121
// Not found, skip.
2222
}
2323
}
@@ -40,7 +40,7 @@ function findWASM (name) {
4040
const root = __dirname
4141
const prebuilds = path.join(root, 'prebuilds')
4242
const folders = readdirSync(prebuilds)
43-
if (folders.find(f => f === name)) {
43+
if (folders.includes(name)) {
4444
return path.join(prebuilds, name, `${name.replaceAll('-', '_')}.js`)
4545
}
4646
}
@@ -76,8 +76,8 @@ function findFolder (root) {
7676

7777
return folders.find(f => f === `${PLATFORM}${LIBC}-${ARCH}`)
7878
|| folders.find(f => f === `${PLATFORM}-${ARCH}`)
79-
} catch (e) {
80-
return null
79+
} catch {
80+
// Ignore
8181
}
8282
}
8383

package.json

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
"cargo-build-release": "yarn -s cargo-build -- --release",
1414
"cargo-build": "cargo build --message-format=json-render-diagnostics",
1515
"copy-artifacts": "node ./scripts/copy-artifacts",
16+
"lint": "eslint .",
1617
"test": "bash scripts/test.sh"
1718
},
1819
"author": "Datadog Inc. <info@datadoghq.com>",
@@ -27,5 +28,14 @@
2728
"homepage": "https://github.com/DataDog/libdatadog-nodejs#readme",
2829
"publishConfig": {
2930
"access": "public"
31+
},
32+
"devDependencies": {
33+
"@eslint/js": "^10.0.1",
34+
"@stylistic/eslint-plugin": "^5.9.0",
35+
"eslint": "^10.0.2",
36+
"eslint-plugin-import-x": "^4.12.2",
37+
"eslint-plugin-n": "^17.24.0",
38+
"eslint-plugin-unicorn": "^63.0.0",
39+
"globals": "^17.4.0"
3040
}
3141
}

scripts/build-wasm.js

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,42 +10,42 @@
1010
// Our releases are built on Linux, and fortunately no special handling is required there. This
1111
// script only allows development to happen on macOS.
1212

13-
const os = require('os');
14-
const childProcess = require('child_process');
13+
const os = require('node:os')
14+
const childProcess = require('node:child_process')
1515

16-
const isMacOS = os.platform() === 'darwin';
17-
const noWasmOpt = isMacOS ? '--no-opt' : '';
18-
const library = process.argv[2];
16+
const isMacOS = os.platform() === 'darwin'
17+
const noWasmOpt = isMacOS ? '--no-opt' : ''
18+
const library = process.argv[2]
1919

2020
const env = {
2121
...process.env,
22-
};
22+
}
2323

2424
if (isMacOS) {
25-
const homebrewDir = env.HOMEBREW_DIR ?? '/opt/homebrew';
26-
const llvmDir = `${homebrewDir}/opt/llvm/`;
27-
const llvmBinDir = `${llvmDir}/bin`;
25+
const homebrewDir = env.HOMEBREW_DIR ?? '/opt/homebrew'
26+
const llvmDir = `${homebrewDir}/opt/llvm/`
27+
const llvmBinDir = `${llvmDir}/bin`
2828

2929
try {
30-
childProcess.execSync(`${llvmBinDir}/llvm-config --version`);
31-
} catch (error) {
32-
console.error(`‼️ LLVM not found in ${llvmDir}.\n‼️ Please install LLVM using Homebrew:\n📝 brew install llvm`);
33-
process.exit(1);
30+
childProcess.execSync(`${llvmBinDir}/llvm-config --version`)
31+
} catch {
32+
console.error(`‼️ LLVM not found in ${llvmDir}.\n‼️ Please install LLVM using Homebrew:\n📝 brew install llvm`)
33+
process.exit(1) // eslint-disable-line unicorn/no-process-exit
3434
}
3535

3636
if (!env.PATH.includes(llvmBinDir)) {
3737
// Add LLVM to PATH if not already included
38-
env.PATH = `${llvmBinDir}:${env.PATH}`;
38+
env.PATH = `${llvmBinDir}:${env.PATH}`
3939
}
4040

4141
// Force C/C++ code (e.g. zstd-sys) to use Homebrew's clang for wasm32. Otherwise a global
4242
// CC (e.g. ccache cc) can point at Apple Clang, which does not support wasm32-unknown-unknown.
43-
env.CC_wasm32_unknown_unknown = `${llvmBinDir}/clang`;
44-
env.CXX_wasm32_unknown_unknown = `${llvmBinDir}/clang++`;
43+
env.CC_wasm32_unknown_unknown = `${llvmBinDir}/clang`
44+
env.CXX_wasm32_unknown_unknown = `${llvmBinDir}/clang++`
4545
}
4646

4747
childProcess.execSync(
4848
`wasm-pack build ${noWasmOpt} --target nodejs ./crates/${library} --out-dir ../../prebuilds/${library}`, {
49-
env
50-
}
51-
);
49+
env,
50+
},
51+
)

scripts/copy-artifacts.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ const outPath = path.join(rootPath, 'target', 'out.ndjson')
1010
const buildPath = path.join(rootPath, 'build', 'Release')
1111

1212
const lineReader = readline.createInterface({
13-
input: fs.createReadStream(outPath)
13+
input: fs.createReadStream(outPath),
1414
})
1515

1616
lineReader.on('line', function (line) {

scripts/test.sh

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,12 @@
22
set -e
33

44
run_test() {
5+
local dir
6+
dir=$(dirname "$1")
7+
if [ -f "${dir}/package.json" ]; then
8+
echo "Installing dependencies for $1"
9+
yarn --cwd "$dir" install
10+
fi
511
echo "Running $1"
612
node "$1"
713
}

test-wasm.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
'use strict'
2+
3+
const fs = require('node:fs')
4+
5+
const crateTestsDir = `./test/wasm/${process.argv[2]}`
6+
const files = fs.readdirSync(crateTestsDir).filter(file => file.endsWith('.js') || !file.includes('.'))
7+
8+
for (const file of files) {
9+
require(`${crateTestsDir}/${file}`)
10+
}

0 commit comments

Comments
 (0)