Skip to content

Commit ed667c7

Browse files
authored
fix(kernel): incorrectly scoped FQN resolutions (#4204)
The registration was done on the object's prototype, and the value from the constructor was always used, even if that was inherited, such that if a parent type had already been resolved previously, all child types would use the parent's FQN instead of their own. Addressed this by instead storing the associations in an external WeakMap, and added a test case to validate correct behavior. Fixes aws/aws-cdk#26604 Fixes #4202 Fixes #4203
1 parent 3232344 commit ed667c7

File tree

2 files changed

+29
-31
lines changed

2 files changed

+29
-31
lines changed

packages/@jsii/kernel/src/objects.test.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { ObjectTable } from './objects';
1+
import { ObjectTable, jsiiTypeFqn } from './objects';
22

33
const mockResolve = jest.fn();
44

@@ -24,3 +24,20 @@ test('registered objects have clean JSON.Stringify', () => {
2424

2525
expect(JSON.stringify(obj)).toEqual(expected);
2626
});
27+
28+
test('FQN resolution is not broken by inheritance relationships', () => {
29+
const rtti = Symbol.for('jsii.rtti');
30+
31+
class Parent {
32+
public static readonly [rtti] = { fqn: 'phony.Parent' };
33+
}
34+
class Child extends Parent {
35+
public static readonly [rtti] = { fqn: 'phony.Child' };
36+
}
37+
38+
const parent = new Parent();
39+
expect(jsiiTypeFqn(parent, () => true)).toBe(Parent[rtti].fqn);
40+
41+
const child = new Child();
42+
expect(jsiiTypeFqn(child, () => true)).toBe(Child[rtti].fqn);
43+
});

packages/@jsii/kernel/src/objects.ts

Lines changed: 11 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -15,31 +15,15 @@ const OBJID_SYMBOL = Symbol.for('$__jsii__objid__$');
1515
*/
1616
const IFACES_SYMBOL = Symbol.for('$__jsii__interfaces__$');
1717

18-
/**
19-
* Symbol we use to tag constructors that are exported from a jsii module.
20-
*/
21-
const JSII_TYPE_FQN_SYMBOL = Symbol('$__jsii__fqn__$');
22-
2318
/**
2419
* Symbol under which jsii runtime type information is stored.
2520
*/
2621
const JSII_RTTI_SYMBOL = Symbol.for('jsii.rtti');
2722

2823
/**
29-
* Exported constructors processed by the jsii compiler have runtime type
30-
* information woven into them under the `[JSII_RTTI]` symbol property.
24+
* Cache for resolved associations between constructors and FQNs.
3125
*/
32-
interface MaybeManagedConstructor {
33-
readonly [JSII_RTTI_SYMBOL]?: {
34-
/** The fully qualified name of the jsii type. */
35-
readonly fqn: spec.FQN;
36-
/** The version of the assembly that declared this type. */
37-
readonly version: string;
38-
};
39-
40-
/** The resolved JSII type FQN for this type. This is set only after resolution */
41-
readonly [JSII_TYPE_FQN_SYMBOL]?: spec.FQN;
42-
}
26+
const RESOLVED_TYPE_FQN = new WeakMap<object, spec.FQN>();
4327

4428
/**
4529
* Get the JSII fqn for an object (if available)
@@ -55,11 +39,11 @@ export function jsiiTypeFqn(
5539
obj: any,
5640
isVisibleType: (fqn: spec.FQN) => boolean,
5741
): spec.FQN | undefined {
58-
const ctor = obj.constructor as MaybeManagedConstructor;
42+
const ctor = obj.constructor;
5943

6044
// We've already resolved for this type, return the cached value.
61-
if (ctor[JSII_TYPE_FQN_SYMBOL] != null) {
62-
return ctor[JSII_TYPE_FQN_SYMBOL];
45+
if (RESOLVED_TYPE_FQN.has(ctor)) {
46+
return RESOLVED_TYPE_FQN.get(ctor)!;
6347
}
6448

6549
let curr = ctor;
@@ -138,22 +122,19 @@ function tagObject(obj: unknown, objid: string, interfaces?: string[]) {
138122
* Set the JSII FQN for classes produced by a given constructor
139123
*/
140124
export function tagJsiiConstructor(constructor: any, fqn: spec.FQN) {
141-
if (Object.prototype.hasOwnProperty.call(constructor, JSII_TYPE_FQN_SYMBOL)) {
125+
const existing = RESOLVED_TYPE_FQN.get(constructor);
126+
127+
if (existing != null) {
142128
return assert.strictEqual(
143-
constructor[JSII_TYPE_FQN_SYMBOL],
129+
existing,
144130
fqn,
145-
`Unable to register ${constructor.name} as ${fqn}: it is already registerd with FQN ${constructor[JSII_TYPE_FQN_SYMBOL]}`,
131+
`Unable to register ${constructor.name} as ${fqn}: it is already registerd with FQN ${existing}`,
146132
);
147133
}
148134

149135
// Mark this constructor as exported from a jsii module, so we know we
150136
// should be considering it's FQN as a valid exported type.
151-
Object.defineProperty(constructor, JSII_TYPE_FQN_SYMBOL, {
152-
configurable: false,
153-
enumerable: false,
154-
writable: false,
155-
value: fqn,
156-
});
137+
RESOLVED_TYPE_FQN.set(constructor, fqn);
157138
}
158139

159140
/**

0 commit comments

Comments
 (0)