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

Bump neo4j-driver to latest version, and update all requisite dependencies #1983

Merged
merged 5 commits into from
Oct 25, 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
3 changes: 3 additions & 0 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,15 @@
"@typescript-eslint/no-this-alias": "warn",
"@typescript-eslint/no-unused-vars": "warn",
"@typescript-eslint/no-var-requires": "warn",
"@typescript-eslint/no-unnecessary-type-constraint": "warn",
"@typescript-eslint/no-loss-of-precision": "warn",
"no-var": "warn",
"prefer-rest-params": "warn",
"prefer-spread": "warn",
"react-hooks/exhaustive-deps": "warn",
"react-hooks/rules-of-hooks": "warn",
"react/no-unescaped-entities": "warn",
"react/no-unknown-property": "warn",
"react/prop-types": "error"
}
},
Expand Down
25 changes: 13 additions & 12 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@
"@types/styled-components": "^5.1.1",
"@types/uuid": "^8.3.0",
"@types/whatwg-url": "^8.2.1",
"@typescript-eslint/eslint-plugin": "^3.9.0",
"@typescript-eslint/parser": "^3.9.0",
"@typescript-eslint/eslint-plugin": "^6.21.0",
"@typescript-eslint/parser": "^6.21.0",
"autoprefixer": "^7.1.4",
"babel-eslint": "^10.0.3",
"babel-jest": "^25.2.4",
Expand All @@ -117,22 +117,22 @@
"cross-env": "^6.0.3",
"css-loader": "^1.0.0",
"cypress": "^8.3.1",
"eslint": "^7.7.0",
"eslint": "^8.57.1",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the version of eslint we were using similarly had a peer dependency on typescript <v4

v9 has some breaking changes with eslint config, again, something we should address, but to keep the change surface low, I've gone with v8

"eslint-config-prettier": "^6.7.0",
"eslint-plugin-import": "^2.18.2",
"eslint-plugin-jsx": "^0.1.0",
"eslint-plugin-node": "^10.0.0",
"eslint-plugin-promise": "^4.2.1",
"eslint-plugin-react": "^7.17.0",
"eslint-plugin-react-hooks": "^4.1.2",
"eslint-plugin-import": "^2.31.0",
"eslint-plugin-react": "^7.37.1",
"eslint-plugin-react-hooks": "^4.6.2",
"express": "^4.15.4",
"file-loader": "^2.0.0",
"fork-ts-checker-notifier-webpack-plugin": "^3.0.0",
"fork-ts-checker-webpack-plugin": "^5.0.14",
"html-loader": "^0.5.5",
"html-webpack-plugin": "^4.5.2",
"husky": "^0.14.3",
"jest": "^25.2.4",
"jest": "^26.6.3",
Copy link
Contributor Author

@daveajrussell daveajrussell Oct 24, 2024

Choose a reason for hiding this comment

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

v25 has a peer dependency of typescript <v4, so we best update the version

however >v27, we run into a host of problems with jestdom and timers that I was unable to resolve

after v27, the jestdom environment is no longer included in the main jest package, and must be imported separately

as soon as this happens, many tests start throwing errors that setImmediate is undefined

there's a lengthy thread here prisma/prisma#8558 with several workarounds

however, of the solutions I've tried, I did not get a successful test run

global.setImmediate = jest.useRealTimers caused tests to hang until they timed out

import { setImmediate } from 'timers;
global.setImmediate = setImmediate

caused any test that relied on RXJS to throw an error that bind was undefined

we also started getting snapshot test failures.. the updates to the snapshots were fairly trivial.. removing Object and Array annotations.. however it was beginning to increase the surface of the change

sticking to v26 negates these issues, although I think we should revisit them soon

Copy link
Contributor

Choose a reason for hiding this comment

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

If we'd want to invest in the build setup for browser I think it'd make sense to go to vite & vitest, but I don't think it's worth the time

"jest-canvas-mock": "^1.1.0",
"js2xmlparser": "^3.0.0",
"json-loader": "^0.5.7",
Expand All @@ -146,18 +146,18 @@
"postcss-loader": "^3.0.0",
"postcss-preset-env": "^3.0.2",
"precss": "^2.0.0",
"prettier-eslint": "^13.0.0",
"prettier-eslint-cli": "^5.0.0",
"prettier-eslint": "^16.3.0",
"prettier-eslint-cli": "^8.0.1",
"raf": "^3.4.0",
"raw-loader": "^0.5.1",
"react-hot-loader": "^4.13.0",
"react-refresh": "^0.11.0",
"react-test-renderer": "^17.0.2",
"redux-mock-store": "^1.2.3",
"style-loader": "^0.23.1",
"ts-jest": "^25.0.0",
"ts-jest": "^26.5.6",
"ts-loader": "^8.0.2",
"typescript": "^3.9.5",
"typescript": "^4.6.3",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

introducing the latest driver version also introduced some type errors at compile time

here, I've set the version of typescript to match that of the neo4j js driver

"typescript-plugin-styled-components": "^1.5.0",
"url-loader": "^1.1.2",
"wait-on": "^3.1.0",
Expand Down Expand Up @@ -206,7 +206,8 @@
"memoize-one": "^5.2.1",
"monaco-editor": "0.23.0",
"neo4j-client-sso": "1.2.3",
"neo4j-driver": "5.9.2",
"neo4j-driver": "5.26.0",
"neo4j-driver-core": "5.26.0",
"re-resizable": "^6.9.9",
"react": "^17.0.2",
"react-dnd": "^11.1.3",
Expand Down
2 changes: 1 addition & 1 deletion src/neo4j-arc/common/types/cypherDataTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export type CypherBasicPropertyType =
| number
| string
| Integer
| BigInt
| bigint
Copy link
Contributor Author

Choose a reason for hiding this comment

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

eslint --fix introduced this change

| Int8Array
| CypherTemporalType
| Point
Expand Down
2 changes: 1 addition & 1 deletion src/neo4j-arc/common/utils/objectUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,6 @@ export function mapObjectValues<A, B>(
)
}

export function keys<T>(object: T): Array<keyof T> {
export function keys<T extends {}>(object: T): Array<keyof T> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixes a compile error where Object.keys(object) doesn't like the generic argument T as is

having T extend object satisfies the param types

Overload 1 of 2, '(o: {}): string[]', gave the following error.
    Argument of type 'T' is not assignable to parameter of type '{}'.
  Overload 2 of 2, '(o: object): string[]', gave the following error.
    Argument of type 'T' is not assignable to parameter of type 'object'.

return Object.keys(object) as Array<keyof T>
}
10 changes: 5 additions & 5 deletions src/shared/modules/commands/cypher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import { version } from 'project-root/package.json'
import { flushPromises } from 'services/utils'

jest.mock('services/bolt/bolt', () => {
const orig = require.requireActual('services/bolt/bolt')
const orig = jest.requireActual('services/bolt/bolt')
return {
...orig,
routedWriteTransaction: jest.fn(() => [
Expand All @@ -39,26 +39,26 @@ jest.mock('services/bolt/bolt', () => {
])
}
})
const bolt = require.requireMock('services/bolt/bolt')
const bolt = jest.requireMock('services/bolt/bolt')

jest.mock('shared/modules/params/paramsDuck', () => {
const orig = require.requireActual('shared/modules/params/paramsDuck')
const orig = jest.requireActual('shared/modules/params/paramsDuck')
return {
...orig,
getParams: () => ({})
}
})

jest.mock('shared/modules/dbMeta/dbMetaDuck', () => {
const orig = require.requireActual('shared/modules/dbMeta/dbMetaDuck')
const orig = jest.requireActual('shared/modules/dbMeta/dbMetaDuck')
return {
...orig,
getRawVersion: () => '3.5.0' // support for tx metadata
}
})

jest.mock('shared/modules/settings/settingsDuck', () => {
const orig = require.requireActual('shared/modules/dbMeta/dbMetaDuck')
const orig = jest.requireActual('shared/modules/dbMeta/dbMetaDuck')
return {
...orig,
shouldUseReadTransactions: () => false
Expand Down
2 changes: 1 addition & 1 deletion src/shared/modules/commands/use-db-ww.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ jest.mock('shared/modules/params/paramsDuck', () => {
})

jest.mock('shared/modules/settings/settingsDuck', () => {
const orig = require.requireActual('shared/modules/dbMeta/dbMetaDuck')
const orig = jest.requireActual('shared/modules/dbMeta/dbMetaDuck')
return {
...orig,
shouldUseReadTransactions: () => false
Expand Down
5 changes: 4 additions & 1 deletion src/shared/services/bolt/driverFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import neo4j, { AuthToken, Config, Driver } from 'neo4j-driver'

import { version } from 'project-root/package.json'
import { isError } from 'shared/utils/typeguards'

export const createDriverOrFailFn = (
url: string,
Expand All @@ -34,7 +35,9 @@ export const createDriverOrFailFn = (
const res = neo4j.driver(url, auth, spreadOpts)
return res
} catch (e) {
failFn(e)
if (isError(e)) {
failFn(e)
}
return null
}
}
7 changes: 5 additions & 2 deletions src/shared/services/bolt/globalDrivers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
toNonRoutingScheme
} from 'services/boltscheme.utils'
import { Connection } from 'shared/modules/connections/connectionsDuck'
import { isBrowserError, isError } from 'shared/utils/typeguards'

interface GlobalDriversObject {
getDirectDriver: () => Driver | null
Expand Down Expand Up @@ -64,8 +65,10 @@ export const buildGlobalDriversObject = async (
routed && (await routed.verifyConnectivity())
routingSupported = true
} catch (e) {
if (e && isNonSupportedRoutingSchemeError(e)) {
routingSupported = false
if (e && isError(e)) {
if (isBrowserError(e) && isNonSupportedRoutingSchemeError(e)) {
routingSupported = false
}
failFn(e)
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/shared/services/commandInterpreterHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ import {
getCurrentDatabase,
isSystemOrCompositeDb
} from 'shared/utils/selectors'
import { isBrowserError } from 'shared/utils/typeguards'

const PLAY_FRAME_TYPES = ['play', 'play-remote']

Expand Down Expand Up @@ -264,7 +265,7 @@ const availableCommands = [
})
)
}
if (action.requestId) {
if (action.requestId && isBrowserError(error)) {
put(updateQueryResult(action.requestId, error, 'error'))
}
}
Expand Down
9 changes: 9 additions & 0 deletions src/shared/utils/typeguards.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { BrowserError } from 'services/exceptions'

export function isError(error: unknown): error is Error {
return error instanceof Error
}

export function isBrowserError(error: unknown): error is BrowserError {
return error instanceof Error && 'code' in error
}
Loading
Loading