Skip to content

Commit 0d2291e

Browse files
taefitaefi
committed
apply suggestions from code review
1 parent 7f01d49 commit 0d2291e

File tree

2 files changed

+60
-65
lines changed

2 files changed

+60
-65
lines changed

‎packages/ts/generator-plugin-signals/src/SignalProcessor.ts

+55-57
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,6 @@ import DependencyManager from '@vaadin/hilla-generator-utils/dependencies/Depend
55
import PathManager from '@vaadin/hilla-generator-utils/dependencies/PathManager.js';
66
import ts, { type FunctionDeclaration, type SourceFile } from 'typescript';
77

8-
export type MethodInfo = Readonly<{
9-
name: string;
10-
signalType: string;
11-
}>;
12-
138
const HILLA_REACT_SIGNALS = '@vaadin/hilla-react-signals';
149

1510
const NUMBER_SIGNAL_CHANNEL = '$NUMBER_SIGNAL_CHANNEL$';
@@ -21,10 +16,10 @@ export default class SignalProcessor {
2116
readonly #dependencyManager: DependencyManager;
2217
readonly #owner: Plugin;
2318
readonly #service: string;
24-
readonly #methods: MethodInfo[];
19+
readonly #methods: Map<string, string>;
2520
readonly #sourceFile: SourceFile;
2621

27-
constructor(service: string, methods: MethodInfo[], sourceFile: SourceFile, owner: Plugin) {
22+
constructor(service: string, methods: Map<string, string>, sourceFile: SourceFile, owner: Plugin) {
2823
this.#service = service;
2924
this.#methods = methods;
3025
this.#sourceFile = sourceFile;
@@ -41,54 +36,68 @@ export default class SignalProcessor {
4136
const [_p, _isType, connectClientId] = imports.default.find((p) => p.includes('connect-client'))!;
4237

4338
this.#processSignalImports(signalImportPaths);
39+
const initTypeId = imports.named.getIdentifier('@vaadin/hilla-frontend', 'EndpointRequestInit');
40+
let initTypeUsageCount = 0;
4441

4542
const [file] = ts.transform<SourceFile>(this.#sourceFile, [
46-
...this.#methods.map((method) =>
47-
transform<SourceFile>((tsNode) => {
48-
if (ts.isFunctionDeclaration(tsNode) && tsNode.name?.text === method.name) {
49-
const body = template(
50-
`
43+
transform((tsNode) => {
44+
if (ts.isFunctionDeclaration(tsNode) && tsNode.name && this.#methods.has(tsNode.name.text)) {
45+
const methodName = tsNode.name.text;
46+
47+
const body = template(
48+
`
5149
function dummy() {
52-
return new ${NUMBER_SIGNAL_CHANNEL}('${this.#service}.${method.name}', ${CONNECT_CLIENT}).signal;
50+
return new ${NUMBER_SIGNAL_CHANNEL}('${this.#service}.${methodName}', ${CONNECT_CLIENT}).signal;
5351
}`,
54-
(statements) => (statements[0] as FunctionDeclaration).body?.statements,
55-
[
56-
transform((node) =>
57-
ts.isIdentifier(node) && node.text === NUMBER_SIGNAL_CHANNEL ? numberSignalChannelId : node,
58-
),
59-
transform((node) => (ts.isIdentifier(node) && node.text === CONNECT_CLIENT ? connectClientId : node)),
60-
],
61-
);
62-
63-
let returnType = tsNode.type;
64-
if (
65-
returnType &&
66-
ts.isTypeReferenceNode(returnType) &&
67-
'text' in returnType.typeName &&
68-
returnType.typeName.text === 'Promise'
69-
) {
70-
if (returnType.typeArguments && returnType.typeArguments.length > 0) {
71-
returnType = returnType.typeArguments[0];
72-
}
52+
(statements) => (statements[0] as FunctionDeclaration).body?.statements,
53+
[
54+
transform((node) =>
55+
ts.isIdentifier(node) && node.text === NUMBER_SIGNAL_CHANNEL ? numberSignalChannelId : node,
56+
),
57+
transform((node) => (ts.isIdentifier(node) && node.text === CONNECT_CLIENT ? connectClientId : node)),
58+
],
59+
);
60+
61+
let returnType = tsNode.type;
62+
if (
63+
returnType &&
64+
ts.isTypeReferenceNode(returnType) &&
65+
'text' in returnType.typeName &&
66+
returnType.typeName.text === 'Promise'
67+
) {
68+
if (returnType.typeArguments && returnType.typeArguments.length > 0) {
69+
returnType = returnType.typeArguments[0];
7370
}
74-
75-
return ts.factory.createFunctionDeclaration(
76-
tsNode.modifiers?.filter((modifier) => modifier.kind !== ts.SyntaxKind.AsyncKeyword),
77-
tsNode.asteriskToken,
78-
tsNode.name,
79-
tsNode.typeParameters,
80-
tsNode.parameters.filter(({ name }) => !(ts.isIdentifier(name) && name.text === 'init')),
81-
returnType,
82-
ts.factory.createBlock(body ?? [], false),
83-
);
8471
}
8572

86-
return tsNode;
87-
}),
88-
),
73+
return ts.factory.createFunctionDeclaration(
74+
tsNode.modifiers?.filter((modifier) => modifier.kind !== ts.SyntaxKind.AsyncKeyword),
75+
tsNode.asteriskToken,
76+
tsNode.name,
77+
tsNode.typeParameters,
78+
tsNode.parameters.filter(({ name }) => !(ts.isIdentifier(name) && name.text === 'init')),
79+
returnType,
80+
ts.factory.createBlock(body ?? [], false),
81+
);
82+
}
83+
return tsNode;
84+
}),
85+
transform((tsNode) => {
86+
if (ts.isFunctionDeclaration(tsNode)) {
87+
if (
88+
!(tsNode.name && this.#methods.has(tsNode.name.text)) &&
89+
tsNode.parameters.some((p) => p.type && ts.isTypeReferenceNode(p.type) && p.type.typeName === initTypeId)
90+
) {
91+
initTypeUsageCount += 1;
92+
}
93+
}
94+
return tsNode;
95+
}),
8996
]).transformed;
9097

91-
this.#removeUnusedRequestInitImports(file);
98+
if (initTypeUsageCount === 0) {
99+
imports.named.remove('@vaadin/hilla-frontend', 'EndpointRequestInit');
100+
}
92101

93102
return createSourceFile(
94103
[
@@ -112,15 +121,4 @@ function dummy() {
112121
}
113122
});
114123
}
115-
116-
#removeUnusedRequestInitImports(file: SourceFile) {
117-
const transformedFileText = ts.createPrinter().printFile(file);
118-
119-
const hasNormalEndpointCalls =
120-
transformedFileText.includes('init?: EndpointRequestInit') && transformedFileText.includes('.call(');
121-
122-
if (!hasNormalEndpointCalls) {
123-
this.#dependencyManager.imports.named.remove('@vaadin/hilla-frontend', 'EndpointRequestInit');
124-
}
125-
}
126124
}

‎packages/ts/generator-plugin-signals/src/index.ts

+5-8
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import Plugin from '@vaadin/hilla-generator-core/Plugin.js';
22
import type SharedStorage from '@vaadin/hilla-generator-core/SharedStorage.js';
33
import type { OpenAPIV3 } from 'openapi-types';
4-
import SignalProcessor, { type MethodInfo } from './SignalProcessor.js';
4+
import SignalProcessor from './SignalProcessor.js';
55

66
export type PathSignalType = Readonly<{
77
path: string;
@@ -33,17 +33,14 @@ function extractEndpointMethodsWithSignalsAsReturnType(storage: SharedStorage):
3333
.reduce<PathSignalType[]>((acc, current) => acc.concat(current), []);
3434
}
3535

36-
function groupByService(signals: readonly PathSignalType[]): Map<string, MethodInfo[]> {
36+
function groupByService(signals: readonly PathSignalType[]): Map<string, Map<string, string>> {
3737
return signals.reduce((serviceMap, signal) => {
3838
const [_, service, method] = signal.path.split('/');
39-
const serviceMethods = serviceMap.get(service) ?? [];
40-
serviceMethods.push({
41-
name: method,
42-
signalType: signal.signalType,
43-
});
39+
const serviceMethods = serviceMap.get(service) ?? new Map<string, string>();
40+
serviceMethods.set(method, signal.signalType);
4441
serviceMap.set(service, serviceMethods);
4542
return serviceMap;
46-
}, new Map<string, MethodInfo[]>());
43+
}, new Map<string, Map<string, string>>());
4744
}
4845

4946
export default class SignalsPlugin extends Plugin {

0 commit comments

Comments
 (0)