JS-937 Support grpc in Node process #6049
Conversation
zglicz
left a comment
zglicz
left a comment
There was a problem hiding this comment.
Good work! Just a few comments.
| import { analyzeFileHandler } from './service.js'; | ||
| import { info, error as logError } from '../../shared/src/helpers/logging.js'; | ||
|
|
||
| const DEFAULT_PORT = 50051; |
There was a problem hiding this comment.
why 50051? when in Docker you have 3000?
| import { info, error as logError } from '../../shared/src/helpers/logging.js'; | ||
|
|
||
| const DEFAULT_PORT = 50051; | ||
| const SERVICE_NAME = 'analyzer.LanguageAnalyzerService'; |
There was a problem hiding this comment.
not something more specific to Js/Ts or Web?
| // Transform gRPC request to analyzeProject input | ||
| const projectInput = transformRequestToProjectInput(request); | ||
|
|
||
| // Call the existing analyzeProject function | ||
| const projectOutput = await analyzeProject(projectInput); | ||
|
|
||
| // Transform the output back to gRPC response format |
There was a problem hiding this comment.
hm... due to the use of nicely named methods, these comments seem superfluous
| } | ||
|
|
||
| const parts = filePaths.map(p => p.split('/')); | ||
| const minLength = Math.min(...parts.map(p => p.length)); |
There was a problem hiding this comment.
oh I didn't know this, I'd assume Math.min(arr) is fine :(
| const filePaths: string[] = []; | ||
|
|
||
| for (const sourceFile of sourceFiles) { | ||
| const relativePath = sourceFile.relativePath ?? ''; |
There was a problem hiding this comment.
hm... seems you get a relative path here? You no trust it?
| } | ||
|
|
||
| // Add rule for both JS and TS to support both languages | ||
| for (const language of ['js', 'ts'] as const) { |
There was a problem hiding this comment.
do you want to check this together with the rspec data to make sure for which language a rule should be enabled?
| } | ||
|
|
||
| return { | ||
| id: `${issue.ruleId}:${filePath}:${issue.line}:${issue.column}`, |
| const params = activeRule.params || []; | ||
| if (params.length > 0) { | ||
| const paramsObj: Record<string, unknown> = {}; | ||
| for (const param of params) { | ||
| // Try to parse numeric values | ||
| const numValue = Number(param.value); | ||
| paramsObj[param.key ?? ''] = !isNaN(numValue) ? numValue : param.value; | ||
| } | ||
| configurations.push(paramsObj); |
There was a problem hiding this comment.
this I'm also a bit curious if it'll work
|
|
||
| const stylelintPkgJson = readFileSync('node_modules/stylelint/package.json', 'utf8'); | ||
|
|
||
| await esbuild.build({ |
There was a problem hiding this comment.
It'd be good if we could have a shared object for esbuild defaults and here we wouldn't need to copy-paste and maintain in 2 places.
2 participants
No description provided.