Skip to content

Commit cd92a30

Browse files
committed
fix(type-compiler): don't emit 'any' for untyped functions, preserve fallback behavior
The #352 fix changed packOpsAndStack to return 'any' for empty ops, which was correct for type aliases but broke arrow functions without explicit type annotations. Functions like `() => this.value` were being wrapped with `__assignType(fn, ['"'])` which caused ReflectionFunction.from() to fail because it expects a function type, not 'any'. The fix adds a getTypeOfFunction method that returns undefined for empty ops (like the pre-#352 behavior), allowing ReflectionFunction.from() to use its fallback path which creates a function type with 'any' return. This preserves the #352 fix for type aliases while fixing the regression for functions. Adds tests for: - Untyped arrow functions in type-compiler - ReflectionFunction.from() with untyped arrow functions in type package
1 parent d8f103b commit cd92a30

File tree

3 files changed

+164
-4
lines changed

3 files changed

+164
-4
lines changed

packages/type-compiler/src/compiler.ts

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2967,11 +2967,31 @@ export class ReflectionTransformer implements CustomTransformer {
29672967
return this.packOpsAndStack(program);
29682968
}
29692969

2970-
protected packOpsAndStack(program: CompilerProgram) {
2970+
/**
2971+
* Like getTypeOfType but returns undefined for empty ops instead of 'any'.
2972+
* Used for functions where empty ops means "can't determine type, don't decorate"
2973+
* rather than "external type, use any".
2974+
*/
2975+
protected getTypeOfFunction(fn: Node | Declaration): Expression | undefined {
2976+
const reflection = this.isWithReflection(this.sourceFile, fn);
2977+
if (!reflection) return;
2978+
2979+
const program = new CompilerProgram(fn, this.sourceFile);
2980+
this.extractPackStructOfType(fn, program);
2981+
return this.packOpsAndStack(program, { emitAnyForEmptyOps: false });
2982+
}
2983+
2984+
protected packOpsAndStack(program: CompilerProgram, options?: { emitAnyForEmptyOps?: boolean }) {
29712985
const packStruct = program.buildPackStruct();
29722986
if (packStruct.ops.length === 0) {
29732987
// External/excluded types produce empty ops - emit 'any' instead of
29742988
// returning undefined which would create invalid JS: `const __ΩType;` (#352)
2989+
// However, for functions, empty ops means "can't determine type" and we should
2990+
// return undefined so the function isn't decorated (this preserves the fallback
2991+
// behavior in ReflectionFunction.from).
2992+
if (options?.emitAnyForEmptyOps === false) {
2993+
return undefined;
2994+
}
29752995
return this.valueToExpression([encodeOps([ReflectionOp.any])]);
29762996
}
29772997
// debugPackStruct(this.sourceFile, program.forNode, packStruct);
@@ -3018,7 +3038,7 @@ export class ReflectionTransformer implements CustomTransformer {
30183038
* => const fn = __assignType(function() {}, [34])
30193039
*/
30203040
protected decorateFunctionExpression(expression: FunctionExpression) {
3021-
const encodedType = this.getTypeOfType(expression);
3041+
const encodedType = this.getTypeOfFunction(expression);
30223042
if (!encodedType) return expression;
30233043

30243044
return this.wrapWithAssignType(expression, encodedType);
@@ -3030,7 +3050,7 @@ export class ReflectionTransformer implements CustomTransformer {
30303050
* => function name() {}; name.__type = 34;
30313051
*/
30323052
protected decorateFunctionDeclaration(declaration: FunctionDeclaration, originalParent?: Node) {
3033-
const encodedType = this.getTypeOfType(declaration);
3053+
const encodedType = this.getTypeOfFunction(declaration);
30343054
if (!encodedType) return declaration;
30353055

30363056
if (!declaration.name) {
@@ -3067,7 +3087,7 @@ export class ReflectionTransformer implements CustomTransformer {
30673087
* => const fn = __assignType(() => {}, [34])
30683088
*/
30693089
protected decorateArrowFunction(expression: ArrowFunction) {
3070-
const encodedType = this.getTypeOfType(expression);
3090+
const encodedType = this.getTypeOfFunction(expression);
30713091
if (!encodedType) return expression;
30723092

30733093
return this.wrapWithAssignType(expression, encodedType);

packages/type-compiler/tests/function-type-hoisting.spec.ts

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -818,3 +818,104 @@ test('transpile: multiple functions all have hoisted __type', () => {
818818
const firstTypeIndex = res.app.indexOf('first.__type');
819819
expect(firstTypeIndex).toBeLessThan(firstFuncIndex);
820820
});
821+
822+
// ============================================================================
823+
// UNTYPED ARROW FUNCTIONS - should not break reflection
824+
// ============================================================================
825+
826+
/**
827+
* Tests for untyped arrow functions that should NOT be decorated with __assignType.
828+
*
829+
* When an arrow function has no explicit type annotations and no parameters,
830+
* the type-compiler should either:
831+
* - Not decorate it at all (original behavior)
832+
* - Or emit a proper function type (function returning any)
833+
*
834+
* The issue was that after fix #352, empty ops were converted to `any` type,
835+
* but this breaks ReflectionFunction.from() which expects a function type.
836+
* See: GitHub Actions CI failure on feat/next branch.
837+
*/
838+
test('runtime: untyped arrow function without parameters still works with ReflectionFunction', () => {
839+
const res = transpileAndRun({
840+
app: `
841+
// Simulate a class with a property accessed via 'this'
842+
class Container {
843+
value = 42;
844+
845+
getFactory() {
846+
// Arrow function with no type annotations referencing 'this'
847+
return () => this.value;
848+
}
849+
}
850+
851+
const container = new Container();
852+
const factory = container.getFactory();
853+
854+
// ReflectionFunction.from should work without throwing
855+
const rf = require('@deepkit/type').ReflectionFunction;
856+
const reflection = rf.from(factory);
857+
858+
// Should get a valid function type (with 'any' return type as fallback)
859+
reflection.type.kind; // Should be ReflectionKind.function (not throw)
860+
`,
861+
});
862+
863+
// ReflectionKind.function = 17
864+
expect(res).toBe(17);
865+
});
866+
867+
test('runtime: untyped arrow function in provider-like pattern works', () => {
868+
const res = transpileAndRun({
869+
app: `
870+
// This simulates the pattern used in @deepkit/app service-container:
871+
// { provide: InjectorContext, useFactory: () => this.injectorContext }
872+
class ServiceContainer {
873+
private contextValue = 'test-context';
874+
875+
addProvider(config: { useFactory: () => any }) {
876+
// The injector calls ReflectionFunction.from on the factory
877+
const rf = require('@deepkit/type').ReflectionFunction;
878+
const reflection = rf.from(config.useFactory);
879+
return reflection.type.kind;
880+
}
881+
}
882+
883+
const container = new ServiceContainer();
884+
// This is the pattern that was breaking
885+
const result = container.addProvider({
886+
useFactory: () => container['contextValue']
887+
});
888+
889+
result;
890+
`,
891+
});
892+
893+
// ReflectionKind.function = 17
894+
expect(res).toBe(17);
895+
});
896+
897+
test('transform: untyped arrow function should not get invalid __assignType', () => {
898+
const res = transform({
899+
app: `
900+
class Container {
901+
value = 42;
902+
getFactory() {
903+
return () => this.value;
904+
}
905+
}
906+
`,
907+
});
908+
909+
// The arrow function should either:
910+
// 1. Not be wrapped with __assignType at all, OR
911+
// 2. Be wrapped with a proper function type (not just 'any')
912+
//
913+
// Invalid: __assignType(() => this.value, ['"'])
914+
// Valid: no __assignType, OR __assignType with proper function bytecode
915+
916+
// Check that if __assignType is present, it doesn't have just ['"'] (which is just 'any')
917+
// The pattern ['"'] means the type is 'any', not 'function returning any'
918+
const hasInvalidAnyOnlyType = res.app.includes("__assignType(() => this.value, ['\"'");
919+
920+
expect(hasInvalidAnyOnlyType).toBe(false);
921+
});

packages/type/tests/integration2.spec.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2168,3 +2168,42 @@ test('map as complex', () => {
21682168
expect(type.types[0].type.types[0].type.types[0].name).toBe('thePayload');
21692169
assertType(type.types[0].type.types[0].type.types[0].type, ReflectionKind.string);
21702170
});
2171+
2172+
test('ReflectionFunction.from() with untyped arrow function', () => {
2173+
// Arrow function without explicit type annotations
2174+
// This pattern is used in DI factory providers like:
2175+
// { provide: InjectorContext, useFactory: () => this.injectorContext }
2176+
const untypedArrowFn = () => 42;
2177+
2178+
// Should not throw - should return a valid function reflection
2179+
const reflection = ReflectionFunction.from(untypedArrowFn);
2180+
2181+
// Should be a function type
2182+
expect(reflection.type.kind).toBe(ReflectionKind.function);
2183+
2184+
// Should have no parameters (since none were declared)
2185+
expect(reflection.getParameters().length).toBe(0);
2186+
2187+
// Return type should be 'any' (since we couldn't infer it)
2188+
expect(reflection.getReturnType().kind).toBe(ReflectionKind.any);
2189+
});
2190+
2191+
test('ReflectionFunction.from() with untyped arrow function referencing this', () => {
2192+
class Container {
2193+
value = 42;
2194+
2195+
getFactory() {
2196+
// This pattern was breaking after #352 fix
2197+
return () => this.value;
2198+
}
2199+
}
2200+
2201+
const container = new Container();
2202+
const factory = container.getFactory();
2203+
2204+
// Should not throw - should return a valid function reflection
2205+
const reflection = ReflectionFunction.from(factory);
2206+
2207+
// Should be a function type (not 'any')
2208+
expect(reflection.type.kind).toBe(ReflectionKind.function);
2209+
});

0 commit comments

Comments
 (0)