Skip to content

Commit 006adf7

Browse files
committed
fix(core): Handle string and closure translation relation targets
`getEntityNamesWithCustomFields()` called every `translations` relation target as a function to read its `.name`. TypeORM also allows a bare string target and a closure returning a string (both used to break circular imports, e.g. `@OneToMany('ArticleTranslation', ...)`). The former threw `relation.type is not a function` — and since this runs unconditionally at the top of `runPluginConfigurations()`, one such relation anywhere in the process aborted every bootstrap; the latter yielded `undefined` and silently failed to exclude the translation entity, re-introducing the duplicate-`customFields` schema error this exclusion exists to prevent. Added `getRelationTargetName()` which resolves all three shapes to the entity name, guarding `typeof type === 'function'` the same way `getEntityTranslation` in validate-custom-fields-config.ts does. Tests cover string, closure-returning- string, and constructor-closure targets. Relates to vendurehq#4965
1 parent ed70212 commit 006adf7

2 files changed

Lines changed: 96 additions & 2 deletions

File tree

packages/core/src/bootstrap.spec.ts

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { describe, expect, it } from 'vitest';
1+
import { getMetadataArgsStorage } from 'typeorm';
2+
import { afterEach, describe, expect, it } from 'vitest';
23

34
import { runPluginConfigurations } from './bootstrap';
45
import { CustomFieldConfig } from './config/custom-field/custom-field-types';
@@ -11,6 +12,37 @@ import { VendurePlugin } from './plugin/vendure-plugin';
1112

1213
void coreEntitiesMap;
1314

15+
/**
16+
* Registers a `translations` relation (and a matching `customFields` embedded on the
17+
* translation target) directly in the TypeORM metadata, so we can exercise the different
18+
* shapes TypeORM allows for a relation target — a constructor closure, a bare string name,
19+
* or a closure returning a string — without declaring throwaway `@Entity` classes that would
20+
* pollute the global metadata for every other test in the process. Returns a cleanup fn.
21+
*/
22+
function registerTranslationRelation(baseName: string, type: unknown): () => void {
23+
const storage = getMetadataArgsStorage();
24+
const base = { name: baseName };
25+
const translationTarget = { name: `${baseName}Translation` };
26+
storage.relations.push({
27+
target: base,
28+
propertyName: 'translations',
29+
relationType: 'one-to-many',
30+
type,
31+
isLazy: false,
32+
options: {},
33+
} as any);
34+
storage.embeddeds.push({
35+
target: translationTarget,
36+
propertyName: 'customFields',
37+
prefix: undefined,
38+
type: () => Object,
39+
} as any);
40+
return () => {
41+
storage.relations.pop();
42+
storage.embeddeds.pop();
43+
};
44+
}
45+
1446
function makeConfig(partial: {
1547
plugins?: RuntimeVendureConfig['plugins'];
1648
customFields?: Record<string, CustomFieldConfig[]>;
@@ -46,6 +78,45 @@ describe('runPluginConfigurations()', () => {
4678
expect(config.customFields.Product).toBe(existing);
4779
});
4880

81+
// OSS-408: a `translations` relation target need not be a constructor closure. TypeORM also
82+
// accepts a bare string name and a closure returning one (both used to break circular imports).
83+
// Building the exclusion set must handle all three, or one such relation anywhere in the
84+
// process throws `relation.type is not a function` and kills every bootstrap (Michael's review).
85+
describe('translation relation target shapes', () => {
86+
let cleanup: (() => void) | undefined;
87+
afterEach(() => {
88+
cleanup?.();
89+
cleanup = undefined;
90+
});
91+
92+
it('does not throw on a string relation target', async () => {
93+
cleanup = registerTranslationRelation('Oss408StringTarget', 'Oss408StringTargetTranslation');
94+
const config = makeConfig({});
95+
await expect(runPluginConfigurations(config)).resolves.toBeDefined();
96+
// and the translation entity is still excluded from auto-init
97+
expect(config.customFields.Oss408StringTargetTranslation).toBeUndefined();
98+
});
99+
100+
it('does not throw on a closure returning a string target, and still excludes it', async () => {
101+
cleanup = registerTranslationRelation(
102+
'Oss408ClosureString',
103+
() => 'Oss408ClosureStringTranslation',
104+
);
105+
const config = makeConfig({});
106+
await expect(runPluginConfigurations(config)).resolves.toBeDefined();
107+
expect(config.customFields.Oss408ClosureStringTranslation).toBeUndefined();
108+
});
109+
110+
it('still excludes a constructor-closure translation target', async () => {
111+
cleanup = registerTranslationRelation('Oss408Closure', () => ({
112+
name: 'Oss408ClosureTranslation',
113+
}));
114+
const config = makeConfig({});
115+
await runPluginConfigurations(config);
116+
expect(config.customFields.Oss408ClosureTranslation).toBeUndefined();
117+
});
118+
});
119+
49120
it('lets a plugin extend a supported entity without a guard', async () => {
50121
@VendurePlugin({
51122
configuration: cfg => {

packages/core/src/entity/register-custom-entity-fields.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
ManyToOne,
1515
} from 'typeorm';
1616
import { EmbeddedMetadataArgs } from 'typeorm/metadata-args/EmbeddedMetadataArgs';
17+
import { RelationMetadataArgs } from 'typeorm/metadata-args/RelationMetadataArgs';
1718
import { DateUtils } from 'typeorm/util/DateUtils';
1819

1920
import { CustomFieldConfig, CustomFields } from '../config/custom-field/custom-field-types';
@@ -49,7 +50,8 @@ export function getEntityNamesWithCustomFields(): string[] {
4950
const translationEntityNames = new Set(
5051
metadataArgsStorage.relations
5152
.filter(relation => relation.propertyName === 'translations')
52-
.map(relation => (relation.type as () => Function)().name),
53+
.map(relation => getRelationTargetName(relation.type))
54+
.filter((name): name is string => name != null),
5355
);
5456
const names = metadataArgsStorage.embeddeds
5557
.filter(embedded => embedded.propertyName === 'customFields')
@@ -58,6 +60,27 @@ export function getEntityNamesWithCustomFields(): string[] {
5860
return Array.from(new Set(names));
5961
}
6062

63+
/**
64+
* Resolves a TypeORM relation target to the target entity's name. The target may be a
65+
* constructor closure (`() => ProductTranslation`, the usual form), a bare string name, or a
66+
* closure returning a string — the latter two are both legal and commonly used to break
67+
* circular imports (`@OneToMany('ArticleTranslation', ...)`). Calling a non-function, as the
68+
* previous code did unconditionally, threw `relation.type is not a function` and aborted
69+
* bootstrap; a closure returning a string yielded `undefined` and silently failed to exclude
70+
* the translation entity.
71+
*/
72+
function getRelationTargetName(type: RelationMetadataArgs['type']): string | undefined {
73+
const resolved: unknown = typeof type === 'function' ? (type as () => unknown)() : type;
74+
if (typeof resolved === 'string') {
75+
return resolved;
76+
}
77+
if (typeof resolved === 'function') {
78+
return resolved.name;
79+
}
80+
// Anything else that carries a `name` (e.g. an EntitySchema-like object).
81+
return (resolved as { name?: string } | undefined)?.name;
82+
}
83+
6184
/**
6285
* Dynamically add columns to the custom field entity based on the CustomFields config.
6386
*/

0 commit comments

Comments
 (0)