-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add a modular graphql server #56
Conversation
b4f4142
to
b6ce433
Compare
bb8cfb0
to
f389034
Compare
export function graphqlModule() { | ||
return ( | ||
/** | ||
* Check if the target class extends RuntimeModule, while |
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.
RuntimeModule -> GraphqlModule
import { GraphqlServer } from "./GraphqlServer"; | ||
import { SchemaGeneratingGraphqlModule } from "./GraphqlModule"; | ||
|
||
export type GraphqlModulesRecord = ModulesRecord<any>; |
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.
Shouldn't this be ModulesRecord<TypedClass<GraphqlModule<unknown>>>
?
public async start(): Promise<void> { | ||
assert(this.graphqlServer !== undefined); | ||
|
||
this.graphqlServer.setContext(this.container); |
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.
setContext
-> setContainer
|
||
const yoga = createYoga<Koa.ParameterizedContext>({ | ||
schema, | ||
graphiql: true, |
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.
things like this should be configurable from above
@@ -10,7 +10,8 @@ | |||
"lint": "eslint ./src ./test", | |||
"test:file": "node --experimental-vm-modules --experimental-wasm-modules --experimental-wasm-threads ../../node_modules/jest/bin/jest.js", | |||
"test": "npm run test:file -- ./src/** ./test/**", | |||
"test:watch": "npm run test:file -- ./src/** ./test/** --watch" | |||
"test:watch": "npm run test:file -- ./src/** ./test/** --watch", | |||
"graphql": "cd ../api && npm run build && cd ../sdk && npm run build && node --experimental-vm-modules --experimental-wasm-modules --experimental-wasm-threads --es-module-specifier-resolution=node ./dist/graphql.js" |
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.
isn't there a way to run this without building? like a watch/dev mode
packages/sdk/src/graphql.ts
Outdated
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.
this looks like a testing leftover, should we move it to ./test or something?
private readonly mempool: Mempool; | ||
|
||
public constructor(@inject("mempool") mempool: Mempool) { | ||
public constructor(@inject("Mempool") mempool: Mempool) { |
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.
maybe public mempool
?
packages/sequencer/package.json
Outdated
@@ -31,18 +31,11 @@ | |||
"@types/node": "^20.2.5" | |||
}, | |||
"dependencies": { | |||
"@types/ws": "^8.5.4", | |||
"@types/koa": "^2.13.10", |
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.
shouldn't this be only a part of the api(-graphql) module?
packages/sdk/src/index.ts
Outdated
export * from "./transaction/AppChainTransaction"; | ||
export * from "./transaction/InMemorySigner"; | ||
export * from "./transaction/InMemoryTransactionSender"; | ||
export * from "./transaction/AuroSigner"; | ||
export * from "./graphql"; |
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.
this should be removed, assuming that file was just for testing purposes
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.
will be in later PR
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.
the reason this is there is bcs prior to using koa, fastify didnt start in a Jest environment, so I had to do this to test stuff
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 the export though
No description provided.