Skip to content

Commit

Permalink
refactor: make cwd required in EngineConfig (#18052)
Browse files Browse the repository at this point in the history
It was already de facto required for `LibraryEngine` (`BinaryEngine`
did handle the missing `cwd`) but it was marked as optional in the
`EngineConfig` interface. There were two places which would break if
`cwd` wasn't passed to the engine constructor:

1. In `DefaultLibraryLoader` it was assumed that `config.cwd` was
   non-nullable.  TypeScript did not raise an error because another
   element in the same array was `any`, which caused the whole array
   to silently be `any[]` and assignable to `string[]`. Fixing that
   issue immediately uncovered a type error in `this.config.cwd` array
   item.

2. In `LibraryEngine` `this.config.cwd` was forcibly converted to a
   non-nullable string with the `!` operator and passed to the query
   engine constructor. This would trigger an error on the Rust side.

Tests in `LibraryEngine.test.ts` didn't have a `cwd` in the engine
config, but they still passed because they used a mock engine and
library loader which didn't rely on `cwd`. They would fail with real
implementations though.

A missing `logEmitter` was added along the way in
`BinaryEngine.test.ts`. TypeScript did flag that as an error, but we
don't type check the old tests.

Ref: https://github.com/prisma/client-planning/issues/103
  • Loading branch information
aqrln committed Feb 23, 2023
1 parent 0153ece commit e1b94ba
Show file tree
Hide file tree
Showing 6 changed files with 10 additions and 5 deletions.
3 changes: 3 additions & 0 deletions packages/client/src/__tests__/binaryEngine.test.ts
@@ -1,5 +1,6 @@
import { BinaryEngine } from '@prisma/engine-core'
import { ClientEngineType, getClientEngineType } from '@prisma/internals'
import { EventEmitter } from 'events'
import path from 'path'

describe('BinaryEngine', () => {
Expand All @@ -16,6 +17,8 @@ describe('BinaryEngine', () => {
datamodelPath: path.join(__dirname, './runtime-tests/blog/schema.prisma'),
tracingConfig: { enabled: false, middleware: false },
env: {},
cwd: process.cwd(),
logEmitter: new EventEmitter(),
})
await engine.start()
} catch (e) {
Expand Down
2 changes: 2 additions & 0 deletions packages/engine-core/src/__tests__/LibraryEngine.test.ts
Expand Up @@ -43,6 +43,7 @@ function setupMockLibraryEngine() {
middleware: false,
},
env: {},
cwd: process.cwd(),
},
loader,
)
Expand Down Expand Up @@ -109,6 +110,7 @@ test('responds to initialization error with PrismaClientInitializationError', as
middleware: false,
},
env: {},
cwd: process.cwd(),
},
loader,
)
Expand Down
4 changes: 2 additions & 2 deletions packages/engine-core/src/binary/BinaryEngine.ts
Expand Up @@ -256,8 +256,8 @@ You may have to run ${chalk.greenBright('prisma generate')} for your changes to
}
}

private resolveCwd(cwd?: string): string {
if (cwd && fs.existsSync(cwd) && fs.lstatSync(cwd).isDirectory()) {
private resolveCwd(cwd: string): string {
if (fs.existsSync(cwd) && fs.lstatSync(cwd).isDirectory()) {
return cwd
}

Expand Down
2 changes: 1 addition & 1 deletion packages/engine-core/src/common/Engine.ts
Expand Up @@ -102,7 +102,7 @@ export interface DatasourceOverwrite {
}

export interface EngineConfig {
cwd?: string
cwd: string
dirname?: string
datamodelPath: string
enableDebugLogs?: boolean
Expand Down
2 changes: 1 addition & 1 deletion packages/engine-core/src/library/DefaultLibraryLoader.ts
Expand Up @@ -144,7 +144,7 @@ Read more about deploying Prisma Client: https://pris.ly/d/client-generator`
return { enginePath, searchedLocations }
}

const dirname = eval('__dirname')
const dirname = eval('__dirname') as string
const searchLocations: string[] = [
// TODO: why hardcoded path? why not look for .prisma/client upwards?
path.resolve(dirname, '../../../.prisma/client'), // Dot Prisma Path
Expand Down
2 changes: 1 addition & 1 deletion packages/engine-core/src/library/LibraryEngine.ts
Expand Up @@ -256,7 +256,7 @@ You may have to run ${chalk.greenBright('prisma generate')} for your changes to
ignoreEnvVarErrors: true,
datasourceOverrides: this.datasourceOverrides,
logLevel: this.logLevel,
configDir: this.config.cwd!,
configDir: this.config.cwd,
},
(log) => {
weakThis.deref()?.logger(log)
Expand Down

0 comments on commit e1b94ba

Please sign in to comment.