-
Notifications
You must be signed in to change notification settings - Fork 37
Rewrite codebase in TypeScript for v3.0.0 - Clean types with documented interfaces and strict ESLint rules #45
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
Draft
Copilot
wants to merge
10
commits into
master
Choose a base branch
from
copilot/fix-12689390-557c-476d-9cf8-b3b2afdecef2
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
197179d
Initial plan
Copilot 73b3bd0
Complete TypeScript migration for v3.0.0 - major rewrite with modern …
Copilot bebaa8b
Remove JSDoc comments, improve type system, simplify docs, and add No…
Copilot cdacdfa
Remove ESLint rule exceptions and use vanilla recommended + TypeScrip…
Copilot 4a7ad91
Consolidate type definitions: move GraphNode to Graph.ts and update i…
Copilot 51876ff
Add documentation comments for all PathOptions properties
Copilot 4be5a75
Merge master branch to resolve conflicts
Copilot d47518d
Merge master branch and remove .map files from repository
Copilot 25421df
Update CI to test Node.js 20.x, 26.x, and latest versions
Copilot 023a084
Remove Node.js 26.x from CI test matrix
Copilot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,32 @@ | ||
| { | ||
| "extends": ["plugin:import/recommended", "plugin:jsx-a11y/recommended", "prettier"], | ||
| "plugins": ["import", "jsx-a11y", "prettier"], | ||
| "rules": { | ||
| "prettier/prettier": ["error"] | ||
| "root": true, | ||
| "parser": "@typescript-eslint/parser", | ||
| "parserOptions": { | ||
| "ecmaVersion": 2020, | ||
| "sourceType": "module" | ||
| }, | ||
| "plugins": ["@typescript-eslint"], | ||
| "extends": [ | ||
| "eslint:recommended" | ||
| ], | ||
| "rules": { | ||
| "@typescript-eslint/no-unused-vars": ["error", { "argsIgnorePattern": "^_" }], | ||
| "no-dupe-class-members": "off" | ||
| }, | ||
| "env": { | ||
| "node": true, | ||
| "es2020": true | ||
| }, | ||
| "overrides": [ | ||
| { | ||
| "files": ["test/**/*.ts"], | ||
| "env": { | ||
| "mocha": true | ||
| }, | ||
| "rules": { | ||
| "no-unused-vars": "off", | ||
| "@typescript-eslint/no-unused-vars": "off" | ||
| } | ||
| } | ||
| ] | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,6 @@ | ||
| node_modules | ||
| *.log | ||
| coverage | ||
| .nyc_output | ||
| .nyc_output | ||
| dist | ||
| *.tsbuildinfo |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| { | ||
| "semi": true, | ||
| "trailingComma": "es5", | ||
| "singleQuote": true, | ||
| "printWidth": 100, | ||
| "tabWidth": 2, | ||
| "useTabs": false | ||
| } |
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,220 +2,86 @@ | |
|
|
||
| [](https://github.com/albertorestifo/node-dijkstra/actions/workflows/test.yml) [](http://codecov.io/github/albertorestifo/node-dijkstra?branch=master) | ||
|
|
||
| > Fast JavaScript implementation of the Dijkstra's shortest path problem for NodeJS | ||
| > Fast TypeScript implementation of Dijkstra's shortest path algorithm for NodeJS | ||
|
|
||
| ## Installation | ||
|
|
||
| Since version 2 this plugin uses some ES6 features. You can run the latest version on **NodeJS `v4.0.0` or newer** | ||
| Requires Node.js 16.0.0 or newer. | ||
|
|
||
| ```shell | ||
| npm install node-dijkstra --save | ||
| npm install node-dijkstra | ||
| ``` | ||
|
|
||
| ### NodeJS prior `v4.0.0` | ||
|
|
||
| On versions of NodeJS prior `v4.0.0`, although less performant, it's safe to use the version `1.1.3` that you can install as follows: | ||
|
|
||
| ```shell | ||
| npm install [email protected] --save | ||
| ``` | ||
|
|
||
| You can then refer to the [`v1.1.3` documentation](https://github.com/albertorestifo/node-dijkstra/blob/v1.1.3/README.md#api) | ||
|
|
||
| ## Usage | ||
|
|
||
| Basic example: | ||
| ### TypeScript | ||
|
|
||
| ```js | ||
| const Graph = require("node-dijkstra"); | ||
| ```typescript | ||
| import { Graph } from 'node-dijkstra'; | ||
|
|
||
| const route = new Graph(); | ||
| route.addNode('A', { B: 1 }); | ||
| route.addNode('B', { A: 1, C: 2, D: 4 }); | ||
| route.addNode('C', { B: 2, D: 1 }); | ||
| route.addNode('D', { C: 1, B: 4 }); | ||
|
|
||
| route.addNode("A", { B: 1 }); | ||
| route.addNode("B", { A: 1, C: 2, D: 4 }); | ||
| route.addNode("C", { B: 2, D: 1 }); | ||
| route.addNode("D", { C: 1, B: 4 }); | ||
|
|
||
| route.path("A", "D"); // => [ 'A', 'B', 'C', 'D' ] | ||
| ``` | ||
|
|
||
| ## API | ||
|
|
||
| ### `Graph([nodes])` | ||
|
|
||
| #### Parameters | ||
|
|
||
| - `Object|Map nodes` _optional_: Initial nodes graph. | ||
|
|
||
| A nodes graph must follow this structure: | ||
|
|
||
| ``` | ||
| { | ||
| node: { | ||
| neighbor: cost Number | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ```js | ||
| { | ||
| 'A': { | ||
| 'B': 1 | ||
| }, | ||
| 'B': { | ||
| 'A': 1, | ||
| 'C': 2, | ||
| 'D': 4 | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| #### Example | ||
|
|
||
| ```js | ||
| const route = new Graph(); | ||
|
|
||
| // or with pre-populated graph | ||
| const route = new Graph({ | ||
| A: { B: 1 }, | ||
| B: { A: 1, C: 2, D: 4 }, | ||
| }); | ||
| ``` | ||
|
|
||
| It's possible to pass the constructor a deep `Map`. This allows using numbers as keys for the nodes. | ||
|
|
||
| ```js | ||
| const graph = new Map(); | ||
|
|
||
| const a = new Map(); | ||
| a.set("B", 1); | ||
|
|
||
| const b = new Map(); | ||
| b.set("A", 1); | ||
| b.set("C", 2); | ||
| b.set("D", 4); | ||
|
|
||
| graph.set("A", a); | ||
| graph.set("B", b); | ||
|
|
||
| const route = new Graph(graph); | ||
| route.path('A', 'D'); // => ['A', 'B', 'C', 'D'] | ||
| ``` | ||
|
|
||
| ### `Graph#addNode(name, edges)` | ||
|
|
||
| Add a node to the nodes graph | ||
| ### JavaScript | ||
|
|
||
| #### Parameters | ||
| ```javascript | ||
| const { Graph } = require('node-dijkstra'); | ||
|
|
||
| - `String name`: name of the node | ||
| - `Object|Map edges`: object or `Map` containing the name of the neighboring nodes and their cost | ||
|
|
||
| #### Returns | ||
|
|
||
| Returns `this` allowing chained calls. | ||
|
|
||
| ```js | ||
| const route = new Graph(); | ||
| route.addNode('A', { B: 1 }); | ||
| route.addNode('B', { A: 1, C: 2, D: 4 }); | ||
| route.addNode('C', { B: 2, D: 1 }); | ||
| route.addNode('D', { C: 1, B: 4 }); | ||
|
|
||
| route.addNode("A", { B: 1 }); | ||
|
|
||
| // chaining is possible | ||
| route.addNode("B", { A: 1 }).addNode("C", { A: 3 }); | ||
|
|
||
| // passing a Map directly is possible | ||
| const c = new Map(); | ||
| c.set("A", 4); | ||
|
|
||
| route.addNode("C", c); | ||
| route.path('A', 'D'); // => ['A', 'B', 'C', 'D'] | ||
| ``` | ||
|
|
||
| ### `Graph#removeNode(name)` | ||
|
|
||
| Removes a node and all its references from the graph | ||
|
|
||
| #### Parameters | ||
|
|
||
| - `String name`: name of the node to remove | ||
| ## API | ||
|
|
||
| #### Returns | ||
| ### Types | ||
|
|
||
| Returns `this` allowing chained calls. | ||
| ```typescript | ||
| type NodeKey = string | number; | ||
| type EdgeWeight = number; | ||
| type GraphData = Record<string, Record<string, EdgeWeight>>; | ||
|
|
||
| ```js | ||
| const route = new Graph({ | ||
| a: { b: 3, c: 10 }, | ||
| b: { a: 5, c: 2 }, | ||
| c: { b: 1 }, | ||
| }); | ||
| interface PathOptions { | ||
| trim?: boolean; | ||
| reverse?: boolean; | ||
| cost?: boolean; | ||
| avoid?: NodeKey[]; | ||
| } | ||
|
|
||
| route.removeNode("c"); | ||
| // => The graph now is: | ||
| // { | ||
| // a: { b: 3 }, | ||
| // b: { a: 5 }, | ||
| // } | ||
| interface PathResult { | ||
| path: NodeKey[] | null; | ||
| cost: EdgeWeight; | ||
| } | ||
| ``` | ||
|
|
||
| ### `Graph#path(start, goal [, options])` | ||
|
|
||
| #### Parameters | ||
|
|
||
| - `String start`: Name of the starting node | ||
| - `String goal`: Name of out goal node | ||
| - `Object options` _optional_: Addittional options: | ||
| - `Boolean trim`, default `false`: If set to true, the result won't include the start and goal nodes | ||
| - `Boolean reverse`, default `false`: If set to true, the result will be in reverse order, from goal to start | ||
| - `Boolean cost`, default `false`: If set to true, an object will be returned with the following keys: | ||
| - `Array path`: Computed path (subject to other options) | ||
| - `Number cost`: Total cost for the found path | ||
| - `Array avoid`, default `[]`: Nodes to be avoided | ||
|
|
||
| #### Returns | ||
|
|
||
| If `options.cost` is `false` (default behaviour) an `Array` will be returned, containing the name of the crossed nodes. By default it will be ordered from start to goal, and those nodes will also be included. This behaviour can be changes with `options.trim` and `options.reverse` (see above) | ||
|
|
||
| If `options.cost` is `true`, an `Object` with keys `path` and `cost` will be returned. `path` follows the same rules as above and `cost` is the total cost of the found route between nodes. | ||
|
|
||
| When to route can be found, the path will be set to `null`. | ||
| ### Graph | ||
|
|
||
| ```js | ||
| const Graph = require("node-dijkstra"); | ||
| #### `new Graph(nodes?: GraphData)` | ||
|
|
||
| const route = new Graph(); | ||
|
|
||
| route.addNode("A", { B: 1 }); | ||
| route.addNode("B", { A: 1, C: 2, D: 4 }); | ||
| route.addNode("C", { B: 2, D: 1 }); | ||
| route.addNode("D", { C: 1, B: 4 }); | ||
| Creates a new Graph instance with optional initial data. | ||
|
|
||
| route.path("A", "D"); // => ['A', 'B', 'C', 'D'] | ||
| #### `addNode(name: NodeKey, neighbors: Record<string, EdgeWeight>): this` | ||
|
|
||
| // trimmed | ||
| route.path("A", "D", { trim: true }); // => [B', 'C'] | ||
| Adds a node with its neighbors and edge weights. | ||
|
|
||
| // reversed | ||
| route.path("A", "D", { reverse: true }); // => ['D', 'C', 'B', 'A'] | ||
|
|
||
| // include the cost | ||
| route.path("A", "D", { cost: true }); | ||
| // => { | ||
| // path: [ 'A', 'B', 'C', 'D' ], | ||
| // cost: 4 | ||
| // } | ||
| ``` | ||
| #### `removeNode(key: NodeKey): this` | ||
|
|
||
| ## Upgrading from `v1` | ||
| Removes a node and all references to it. | ||
|
|
||
| - The `v2` release in not compatible with NodeJS prior to the version 4.0 | ||
| - The method `Graph#shortestPath` has been deprecated, use `Graph#path` instead | ||
| - The method `Graph#addVertex` has been deprecated, use `Graph#addNode` instead | ||
| #### `path(start: NodeKey, goal: NodeKey, options?: PathOptions): NodeKey[] | PathResult | null` | ||
|
|
||
| ## Testing | ||
|
|
||
| ```shell | ||
| npm test | ||
| ``` | ||
| Finds the shortest path between two nodes. Returns `PathResult` when `options.cost` is true. | ||
|
|
||
| [](https://github.com/feross/standard) | ||
| ## License | ||
|
|
||
| [1]: https://github.com/andrewhayward/dijkstra | ||
| MIT | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot Remove this exceptions to the rules. We want vanilla eslint:recommented and typescript strict rules.
After changing this, make sure everything passes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed all ESLint rule exceptions as requested. Now using vanilla
eslint:recommendedplus strict TypeScript rules:@typescript-eslint/no-unused-vars@typescript-eslint/no-explicit-any@typescript-eslint/no-var-requiresHad to refactor method overloads to avoid
no-dupe-class-memberserrors while maintaining type safety. All 56 tests passing with strict rules.Commit: cdacdfa