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

Linter fix #2263

Merged
merged 1 commit into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions .github/workflows/lint-ts.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ jobs:

- name: Install dependencies
run: |
npm install eslint-plugin-import @eslint/compat globals @eslint/js @eslint/eslintrc @typescript-eslint/parser @typescript-eslint/eslint-plugin eslint-plugin-tsdoc eslint typescript eslint-config-prettier prettier
npm install

- name: Run linting and prettier
run: |
for folder in node benchmarks/node benchmarks/utilities; do
npx eslint ${{ github.workspace }}/$folder
npx eslint ${{ github.workspace }}/$folder -c ${{ github.workspace }}/eslint.config.mjs
npx prettier --check ${{ github.workspace }}/$folder
done
5 changes: 2 additions & 3 deletions .github/workflows/lint-ts/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,11 @@ runs:
steps:
- uses: actions/checkout@v4

- run: cp eslint.config.mjs ${{ inputs.package-folder }}
- run: cp eslint.config.mjs ${{ inputs.package-folder }}
shell: bash

- run: |
npm install eslint-plugin-import @eslint/compat globals @eslint/js @eslint/eslintrc @typescript-eslint/parser @typescript-eslint/eslint-plugin eslint-plugin-tsdoc eslint typescript eslint-config-prettier
npm i
npm install
npx eslint . --max-warnings=0
working-directory: ${{ inputs.package-folder }}
shell: bash
60 changes: 18 additions & 42 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
@@ -1,48 +1,21 @@
import { fixupConfigRules, fixupPluginRules } from "@eslint/compat";
import typescriptEslint from "@typescript-eslint/eslint-plugin";
import tsdoc from "eslint-plugin-tsdoc";
import _import from "eslint-plugin-import";
import globals from "globals";
import tsParser from "@typescript-eslint/parser";
import path from "node:path";
import { fileURLToPath } from "node:url";
import js from "@eslint/js";
import { FlatCompat } from "@eslint/eslintrc";
// @ts-check
import eslint from '@eslint/js';
import prettierConfig from 'eslint-config-prettier';
import tseslint from 'typescript-eslint';

const __filename = fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);
const compat = new FlatCompat({
baseDirectory: __dirname,
recommendedConfig: js.configs.recommended,
allConfig: js.configs.all
});

export default [...fixupConfigRules(compat.extends(
"eslint:recommended",
"plugin:@typescript-eslint/recommended",
"plugin:import/errors",
"plugin:import/warnings",
)), {
plugins: {
"@typescript-eslint": fixupPluginRules(typescriptEslint),
tsdoc,
import: fixupPluginRules(_import),
export default tseslint.config(
eslint.configs.recommended,
...tseslint.configs.recommended,
...tseslint.configs.stylistic,
{ files: ['**/*.js'],
...tseslint.configs.disableTypeChecked,
},

languageOptions: {
globals: {
...globals.browser,
...globals.node,
...globals.jest,
},

parser: tsParser,
{
ignores: ["*/ProtobufMessage.*", "**/*.d.ts", "node_modules/**", "build-ts/**", "jest.config.js", ],
},

rules: {
"tsdoc/syntax": "error",
{rules: {
"import/no-unresolved": "off",

"padding-line-between-statements": ["error", {
blankLine: "always",
prev: "*",
Expand All @@ -68,5 +41,8 @@ export default [...fixupConfigRules(compat.extends(
prev: "multiline-block-like",
next: "*",
}],
},
}];
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we lint this file as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Mmm i guess?

},
prettierConfig,

);
20 changes: 4 additions & 16 deletions node/DEVELOPER.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,29 +164,17 @@ Development on the Node wrapper may involve changes in either the TypeScript or

```bash
# Run from the root folder of the GLIDE repository
npm install @typescript-eslint/parser @typescript-eslint/eslint-plugin eslint-plugin-tsdoc eslint typescript eslint-config-prettier prettier
npm i
npm install --save-dev prettier
cd node
npx eslint . --max-warnings=0
npx prettier --check .
```

To automatically apply prettier recommendations, run the following command:

```bash
npm run lint
# To automatically apply ESLint and/or prettier recommendations
npx run lint:fix
npx prettier -w .
```

To avoid getting ESLint warnings from protobuf generated files, run the following command:

```bash
npx eslint --ignore-pattern ProtobufMessage.* .
```

To automatically apply ESLint recommendations, run the following command:

```bash
npx eslint --ignore-pattern ProtobufMessage.* --ignore-pattern 'build-ts/**' --fix .
```

2. Rust
Expand Down
1 change: 0 additions & 1 deletion node/babel.config.js

This file was deleted.

4 changes: 2 additions & 2 deletions node/hybrid-node-tests/commonjs-test/commonjs-test.cjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint-disable @typescript-eslint/no-var-requires */
/* eslint-disable @typescript-eslint/no-require-imports */
/* eslint no-undef: off */
/* eslint @typescript-eslint/no-require-imports: off */
const { AsyncClient } = require("glide-rs");
const RedisServer = require("redis-server");
const FreePort = require("find-free-port");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint no-undef: off */
import { execFile } from "child_process";
import findFreePorts from "find-free-ports";
import { AsyncClient } from "glide-rs";
Expand Down
1 change: 1 addition & 0 deletions node/jest.config.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint no-undef: off */
module.exports = {
preset: "ts-jest",
transform: { "^.+\\.ts?$": "ts-jest" },
Expand Down
9 changes: 4 additions & 5 deletions node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
},
"homepage": "https://github.com/valkey-io/valkey-glide#readme",
"dependencies": {
"@eslint/compat": "^1.1.1",
"eslint-config-prettier": "^9.1.0",
"glide-rs": "file:rust-client",
"long": "^5.2.3",
Expand All @@ -35,7 +36,9 @@
"fix-protobuf-file": "replace 'this\\.encode\\(message, writer\\)\\.ldelim' 'this.encode(message, writer && writer.len ? writer.fork() : writer).ldelim' src/ProtobufMessage.js",
"test": "npm run build-test-utils && jest --verbose --runInBand --testPathIgnorePatterns='RedisModules'",
"build-test-utils": "cd ../utils && npm i && npm run build",
"lint": "eslint -f unix \"src/**/*.{ts,tsx}\"",
"lint:fix": "npm run install-linting && npx eslint -c ../eslint.config.mjs --fix",
"lint": "npm run install-linting && npx eslint -c ../eslint.config.mjs",
Comment on lines +39 to +40
Copy link
Collaborator

Choose a reason for hiding this comment

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

These should include prettier as well

Copy link
Member Author

Choose a reason for hiding this comment

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

Thought about it, didn't know if do or not. If you think so its enough for me, will do.

"install-linting": "cd ../ & npm install",
"prepack": "npmignore --auto",
"prettier:check:ci": "./node_modules/.bin/prettier --check . --ignore-unknown '!**/*.{js,d.ts}'",
"prettier:format": "./node_modules/.bin/prettier --write . --ignore-unknown '!**/*.{js,d.ts}'"
Expand All @@ -48,11 +51,7 @@
"@types/redis-server": "^1.2.2",
"@types/semver": "^7.5.8",
"@types/uuid": "^10.0.0",
"@typescript-eslint/eslint-plugin": "^8.4.0",
"@typescript-eslint/parser": "^8.4.0",
"babel-jest": "^29.7.0",
"eslint": "^9.10.0",
"eslint-plugin-tsdoc": "^0.3.0",
"find-free-port": "^2.0.0",
"jest": "^29.7.0",
"jest-html-reporter": "^3.10.2",
Expand Down
8 changes: 1 addition & 7 deletions node/rust-client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,24 +40,18 @@
"format": "run-p format:prettier format:rs",
"format:prettier": "prettier . -w",
"format:rs": "cargo fmt",
"lint": "eslint . -c ./.eslintrc.yml",
"prepublishOnly": "napi prepublish -t npm",
"version": "napi version"
},
"devDependencies": {
"@napi-rs/cli": "^2.18.4",
"@typescript-eslint/eslint-plugin": "^8.4.0",
"@typescript-eslint/parser": "^8.4.0",
"eslint": "^9.10.0",
"eslint-config-prettier": "^9.1.0",
"eslint-plugin-prettier": "^5.2.1",
"lint-staged": "^15.2.10",
"npm-run-all": "^4.1.5",
"prettier": "^3.3.3"
},
"lint-staged": {
"*.@(js|ts|tsx)": [
"eslint -c .eslintrc.yml --fix"
"npx eslint -c ../../eslint.config.mjs --fix"
],
"*.@(js|ts|tsx|yml|yaml|md|json)": [
"prettier --write"
Expand Down
27 changes: 14 additions & 13 deletions node/src/BaseClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,12 +250,13 @@ import {
/* eslint-disable-next-line @typescript-eslint/no-explicit-any */
type PromiseFunction = (value?: any) => void;
type ErrorFunction = (error: RedisError) => void;
/* eslint @typescript-eslint/consistent-indexed-object-style: off, @typescript-eslint/consistent-type-definitions: off */
export type ReturnTypeRecord = { [key: string]: ReturnType };
export type ReturnTypeMap = Map<string, ReturnType>;
export type ReturnTypeAttribute = {
export interface ReturnTypeAttribute {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why you changed to interface? did you changed it to all types?

Copy link
Member Author

Choose a reason for hiding this comment

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

Interface is recommended by ts since it's behave like the JS under the hood and led to better performance, unless there's a specific need the recommendation is to use interface.

Copy link
Collaborator

Choose a reason for hiding this comment

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

note that it is legal in TypeScript for multiple interfaces with the same name to be created. TypeScript will merge interfaces with the same name. Types cannot be merged. If you try to define the same type twice, TypeScript will throw an error.

value: ReturnType;
attributes: ReturnTypeRecord;
};
}
export enum ProtocolVersion {
/** Use RESP2 to communicate with the server nodes. */
RESP2 = connection_request.ProtocolVersion.RESP2,
Expand Down Expand Up @@ -296,13 +297,13 @@ export enum Decoder {
}

/** An extension to command option types with {@link Decoder}. */
export type DecoderOption = {
export interface DecoderOption {
/**
* {@link Decoder} type which defines how to handle the response.
* If not set, the {@link BaseClientConfiguration.defaultDecoder|default decoder} will be used.
*/
decoder?: Decoder;
};
}

/** A replacement for `Record<GlideString, T>` - array of key-value pairs. */
export type GlideRecord<T> = {
Expand Down Expand Up @@ -411,7 +412,7 @@ class PointerResponse {
}

/** Represents the credentials for connecting to a server. */
export type RedisCredentials = {
export interface RedisCredentials {
/**
* The username that will be used for authenticating connections to the Valkey servers.
* If not supplied, "default" will be used.
Expand All @@ -421,7 +422,7 @@ export type RedisCredentials = {
* The password that will be used for authenticating connections to the Valkey servers.
*/
password: string;
};
}

/** Represents the client's read from strategy. */
export type ReadFrom =
Expand All @@ -434,7 +435,7 @@ export type ReadFrom =
/**
* Configuration settings for creating a client. Shared settings for standalone and cluster clients.
*/
export type BaseClientConfiguration = {
export interface BaseClientConfiguration {
/**
* DNS Addresses and ports of known nodes in the cluster.
* If the server is in cluster mode the list can be partial, as the client will attempt to map out the cluster and find all nodes.
Expand Down Expand Up @@ -495,9 +496,9 @@ export type BaseClientConfiguration = {
* If not set, 'Decoder.String' will be used.
*/
defaultDecoder?: Decoder;
};
}

export type ScriptOptions = {
export interface ScriptOptions {
/**
* The keys that are used in the script.
*/
Expand All @@ -506,7 +507,7 @@ export type ScriptOptions = {
* The arguments for the script.
*/
args?: GlideString[];
};
}

function getRequestErrorClass(
type: response.RequestErrorType | null | undefined,
Expand Down Expand Up @@ -604,11 +605,11 @@ function toProtobufRoute(
}
}

export type PubSubMsg = {
export interface PubSubMsg {
message: string;
channel: string;
pattern?: string | null;
};
}

/**
* @internal
Expand Down Expand Up @@ -4234,7 +4235,7 @@ export class BaseClient {
destination: GlideString,
source: GlideString,
rangeQuery: RangeByScore | RangeByLex | RangeByIndex,
reverse: boolean = false,
reverse = false,
): Promise<number> {
return this.createWritePromise(
createZRangeStore(destination, source, rangeQuery, reverse),
Expand Down
Loading
Loading