Skip to content

Commit 61cb3a4

Browse files
RomainMullerRomainMuller
authored
fix(kernel): Return object literals as references (#249)
Use the javascript `Proxy` class to coalesce object literals to an interface type, allowing them to be returned by reference instead of by value. Introduces a test in the `jsii-kernel` to demonstrate the feature works as intended. Fixes #248 Fixes aws/aws-cdk#774
1 parent 96ac5d6 commit 61cb3a4

File tree

17 files changed

+517
-11
lines changed

17 files changed

+517
-11
lines changed

‎packages/jsii-calc/lib/compliance.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -887,3 +887,11 @@ export class AbstractClassReturner {
887887
}
888888
}
889889
}
890+
891+
export interface MutableObjectLiteral {
892+
value: string;
893+
}
894+
895+
export class ClassWithMutableObjectLiteralProperty {
896+
public mutableObject: MutableObjectLiteral = { value: 'default' };
897+
}

‎packages/jsii-calc/test/assembly.jsii

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1018,6 +1018,23 @@
10181018
}
10191019
]
10201020
},
1021+
"jsii-calc.ClassWithMutableObjectLiteralProperty": {
1022+
"assembly": "jsii-calc",
1023+
"fqn": "jsii-calc.ClassWithMutableObjectLiteralProperty",
1024+
"initializer": {
1025+
"initializer": true
1026+
},
1027+
"kind": "class",
1028+
"name": "ClassWithMutableObjectLiteralProperty",
1029+
"properties": [
1030+
{
1031+
"name": "mutableObject",
1032+
"type": {
1033+
"fqn": "jsii-calc.MutableObjectLiteral"
1034+
}
1035+
}
1036+
]
1037+
},
10211038
"jsii-calc.DefaultedConstructorArgument": {
10221039
"assembly": "jsii-calc",
10231040
"fqn": "jsii-calc.DefaultedConstructorArgument",
@@ -1883,6 +1900,22 @@
18831900
}
18841901
]
18851902
},
1903+
"jsii-calc.MutableObjectLiteral": {
1904+
"assembly": "jsii-calc",
1905+
"datatype": true,
1906+
"fqn": "jsii-calc.MutableObjectLiteral",
1907+
"kind": "interface",
1908+
"name": "MutableObjectLiteral",
1909+
"properties": [
1910+
{
1911+
"abstract": true,
1912+
"name": "value",
1913+
"type": {
1914+
"primitive": "string"
1915+
}
1916+
}
1917+
]
1918+
},
18861919
"jsii-calc.Negate": {
18871920
"assembly": "jsii-calc",
18881921
"base": {
@@ -3230,5 +3263,5 @@
32303263
}
32313264
},
32323265
"version": "0.7.6",
3233-
"fingerprint": "ecDtx3DHVZi7UjyCDhGncg4jbSRaD536bUyh6YzAxlY="
3266+
"fingerprint": "DFShLmIJQmPjTv5Dmz4JoBKqoCtAyifD1RpCuHo+sEc="
32343267
}

‎packages/jsii-kernel/lib/kernel.ts

Lines changed: 129 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import { TOKEN_DATE, TOKEN_ENUM, TOKEN_REF } from './api';
1414
*/
1515
const OBJID_PROP = '$__jsii__objid__$';
1616
const FQN_PROP = '$__jsii__fqn__$';
17+
const PROXIES_PROP = '$__jsii__proxies__$';
18+
const PROXY_REFERENT_PROP = '$__jsii__proxy_referent__$';
1719

1820
/**
1921
* A special FQN that can be used to create empty javascript objects.
@@ -149,9 +151,14 @@ export class Kernel {
149151
const { objref } = req;
150152

151153
this._debug('del', objref);
152-
this._findObject(objref); // make sure object exists
154+
const obj = this._findObject(objref); // make sure object exists
153155
delete this.objects[objref[TOKEN_REF]];
154156

157+
if (obj[PROXY_REFERENT_PROP]) {
158+
// De-register the proxy if this was a proxy...
159+
delete obj[PROXY_REFERENT_PROP][PROXIES_PROP][obj[FQN_PROP]];
160+
}
161+
155162
return { };
156163
}
157164

@@ -923,12 +930,20 @@ export class Kernel {
923930
// so the client receives a real object.
924931
if (typeof(v) === 'object' && targetType && spec.isNamedTypeReference(targetType)) {
925932
this._debug('coalescing to', targetType);
926-
const newObjRef = this._create({ fqn: targetType.fqn });
927-
const newObj = this._findObject(newObjRef);
928-
for (const k of Object.keys(v)) {
929-
newObj[k] = v[k];
933+
/*
934+
* We "cache" proxy instances in [PROXIES_PROP] so we can return an
935+
* identical object reference upon multiple accesses of the same
936+
* object literal under the same exposed type. This results in a
937+
* behavior that is more consistent with class instances.
938+
*/
939+
const proxies: Proxies = v[PROXIES_PROP] = v[PROXIES_PROP] || {};
940+
if (!proxies[targetType.fqn]) {
941+
const handler = new KernelProxyHandler(v);
942+
const proxy = new Proxy(v, handler);
943+
// _createObjref will set the FQN_PROP & OBJID_PROP on the proxy.
944+
proxies[targetType.fqn] = { objRef: this._createObjref(proxy, targetType.fqn), handler };
930945
}
931-
return newObjRef;
946+
return proxies[targetType.fqn].objRef;
932947
}
933948

934949
// date (https://stackoverflow.com/a/643827/737957)
@@ -1135,3 +1150,111 @@ function mapSource(err: Error, sourceMaps: { [assm: string]: SourceMapConsumer }
11351150
return frame;
11361151
}
11371152
}
1153+
1154+
type ObjectKey = string | number | symbol;
1155+
/**
1156+
* A Proxy handler class to support mutation of the returned object literals, as
1157+
* they may "embody" several different interfaces. The handler is in particular
1158+
* responsible to make sure the ``FQN_PROP`` and ``OBJID_PROP`` do not get set
1159+
* on the ``referent`` object, for this would cause subsequent accesses to
1160+
* possibly return incorrect object references.
1161+
*/
1162+
class KernelProxyHandler implements ProxyHandler<any> {
1163+
private readonly ownProperties: { [key: string]: any } = {};
1164+
1165+
/**
1166+
* @param referent the "real" value that will be returned.
1167+
*/
1168+
constructor(public readonly referent: any) {
1169+
/*
1170+
* Proxy-properties must exist as non-configurable & writable on the
1171+
* referent, otherwise the Proxy will not allow returning ``true`` in
1172+
* response to ``defineProperty``.
1173+
*/
1174+
for (const prop of [FQN_PROP, OBJID_PROP]) {
1175+
Object.defineProperty(referent, prop, {
1176+
configurable: false,
1177+
enumerable: false,
1178+
writable: true,
1179+
value: undefined
1180+
});
1181+
}
1182+
}
1183+
1184+
public defineProperty(target: any, property: ObjectKey, attributes: PropertyDescriptor): boolean {
1185+
switch (property) {
1186+
case FQN_PROP:
1187+
case OBJID_PROP:
1188+
return Object.defineProperty(this.ownProperties, property, attributes);
1189+
default:
1190+
return Object.defineProperty(target, property, attributes);
1191+
}
1192+
}
1193+
1194+
public deleteProperty(target: any, property: ObjectKey): boolean {
1195+
switch (property) {
1196+
case FQN_PROP:
1197+
case OBJID_PROP:
1198+
delete this.ownProperties[property];
1199+
break;
1200+
default:
1201+
delete target[property];
1202+
}
1203+
return true;
1204+
}
1205+
1206+
public getOwnPropertyDescriptor(target: any, property: ObjectKey): PropertyDescriptor | undefined {
1207+
switch (property) {
1208+
case FQN_PROP:
1209+
case OBJID_PROP:
1210+
return Object.getOwnPropertyDescriptor(this.ownProperties, property);
1211+
default:
1212+
return Object.getOwnPropertyDescriptor(target, property);
1213+
}
1214+
}
1215+
1216+
public get(target: any, property: ObjectKey): any {
1217+
switch (property) {
1218+
// Magical property for the proxy, so we can tell it's one...
1219+
case PROXY_REFERENT_PROP:
1220+
return this.referent;
1221+
case FQN_PROP:
1222+
case OBJID_PROP:
1223+
return this.ownProperties[property];
1224+
default:
1225+
return target[property];
1226+
}
1227+
}
1228+
1229+
public set(target: any, property: ObjectKey, value: any): boolean {
1230+
switch (property) {
1231+
case FQN_PROP:
1232+
case OBJID_PROP:
1233+
this.ownProperties[property] = value;
1234+
break;
1235+
default:
1236+
target[property] = value;
1237+
}
1238+
return true;
1239+
}
1240+
1241+
public has(target: any, property: ObjectKey): boolean {
1242+
switch (property) {
1243+
case FQN_PROP:
1244+
case OBJID_PROP:
1245+
return property in this.ownProperties;
1246+
default:
1247+
return property in target;
1248+
}
1249+
}
1250+
1251+
public ownKeys(target: any): ObjectKey[] {
1252+
return Reflect.ownKeys(target).concat(Reflect.ownKeys(this.ownProperties));
1253+
}
1254+
}
1255+
1256+
type Proxies = { [fqn: string]: ProxyReference };
1257+
interface ProxyReference {
1258+
objRef: api.ObjRef;
1259+
handler: KernelProxyHandler;
1260+
}

‎packages/jsii-kernel/test/test.kernel.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -873,6 +873,23 @@ defineTest('node.js standard library', async (test, sandbox) => {
873873
{ result: "6a2da20943931e9834fc12cfe5bb47bbd9ae43489a30726962b576f4e3993e50" });
874874
});
875875

876+
// @see awslabs/jsii#248
877+
defineTest('object literals are returned by reference', async (test, sandbox) => {
878+
const objref = sandbox.create({ fqn: 'jsii-calc.ClassWithMutableObjectLiteralProperty' });
879+
const property = sandbox.get({ objref, property: 'mutableObject' }).value;
880+
881+
const newValue = 'Bazinga!1!';
882+
sandbox.set({ objref: property, property: 'value', value: newValue });
883+
884+
test.equal(newValue,
885+
sandbox.get({
886+
objref: sandbox.get({ objref, property: 'mutableObject' }).value,
887+
property: 'value'
888+
}).value);
889+
890+
sandbox.del({ objref: property });
891+
});
892+
876893
const testNames: { [name: string]: boolean } = { };
877894

878895
async function createCalculatorSandbox(name: string) {

‎packages/jsii-kernel/tsconfig.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@
4444
/* Source Map Options */
4545
// "sourceRoot": "./", /* Specify the location where debugger should locate TypeScript files instead of source locations. */
4646
// "mapRoot": "./", /* Specify the location where debugger should locate map files instead of generated locations. */
47-
// "inlineSourceMap": false, /* Emit a single file with source maps instead of having a separate file. */
48-
// "inlineSources": false, /* Emit the source alongside the sourcemaps within a single file; requires '--inlineSourceMap' or '--sourceMap' to be set. */
47+
"inlineSourceMap": true, /* Emit a single file with source maps instead of having a separate file. */
48+
"inlineSources": true, /* Emit the source alongside the sourcemaps within a single file; requires '--inlineSourceMap' or '--sourceMap' to be set. */
4949

5050
/* Experimental Options */
5151
"experimentalDecorators": true /* Enables experimental support for ES7 decorators. */

‎packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/.jsii

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1018,6 +1018,23 @@
10181018
}
10191019
]
10201020
},
1021+
"jsii-calc.ClassWithMutableObjectLiteralProperty": {
1022+
"assembly": "jsii-calc",
1023+
"fqn": "jsii-calc.ClassWithMutableObjectLiteralProperty",
1024+
"initializer": {
1025+
"initializer": true
1026+
},
1027+
"kind": "class",
1028+
"name": "ClassWithMutableObjectLiteralProperty",
1029+
"properties": [
1030+
{
1031+
"name": "mutableObject",
1032+
"type": {
1033+
"fqn": "jsii-calc.MutableObjectLiteral"
1034+
}
1035+
}
1036+
]
1037+
},
10211038
"jsii-calc.DefaultedConstructorArgument": {
10221039
"assembly": "jsii-calc",
10231040
"fqn": "jsii-calc.DefaultedConstructorArgument",
@@ -1883,6 +1900,22 @@
18831900
}
18841901
]
18851902
},
1903+
"jsii-calc.MutableObjectLiteral": {
1904+
"assembly": "jsii-calc",
1905+
"datatype": true,
1906+
"fqn": "jsii-calc.MutableObjectLiteral",
1907+
"kind": "interface",
1908+
"name": "MutableObjectLiteral",
1909+
"properties": [
1910+
{
1911+
"abstract": true,
1912+
"name": "value",
1913+
"type": {
1914+
"primitive": "string"
1915+
}
1916+
}
1917+
]
1918+
},
18861919
"jsii-calc.Negate": {
18871920
"assembly": "jsii-calc",
18881921
"base": {
@@ -3230,5 +3263,5 @@
32303263
}
32313264
},
32323265
"version": "0.7.6",
3233-
"fingerprint": "ecDtx3DHVZi7UjyCDhGncg4jbSRaD536bUyh6YzAxlY="
3266+
"fingerprint": "DFShLmIJQmPjTv5Dmz4JoBKqoCtAyifD1RpCuHo+sEc="
32343267
}

‎packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/ClassWithMutableObjectLiteralProperty.cs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
using Amazon.JSII.Runtime.Deputy;
2+
3+
namespace Amazon.JSII.Tests.CalculatorNamespace
4+
{
5+
[JsiiClass(typeof(ClassWithMutableObjectLiteralProperty), "jsii-calc.ClassWithMutableObjectLiteralProperty", "[]")]
6+
public class ClassWithMutableObjectLiteralProperty : DeputyBase
7+
{
8+
public ClassWithMutableObjectLiteralProperty(): base(new DeputyProps(new object[]{}))
9+
{
10+
}
11+
12+
protected ClassWithMutableObjectLiteralProperty(ByRefValue reference): base(reference)
13+
{
14+
}
15+
16+
protected ClassWithMutableObjectLiteralProperty(DeputyProps props): base(props)
17+
{
18+
}
19+
20+
[JsiiProperty("mutableObject", "{\"fqn\":\"jsii-calc.MutableObjectLiteral\"}")]
21+
public virtual IMutableObjectLiteral MutableObject
22+
{
23+
get => GetInstanceProperty<IMutableObjectLiteral>();
24+
set => SetInstanceProperty(value);
25+
}
26+
}
27+
}

‎packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/IMutableObjectLiteral.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
using Amazon.JSII.Runtime.Deputy;
2+
3+
namespace Amazon.JSII.Tests.CalculatorNamespace
4+
{
5+
[JsiiInterface(typeof(IMutableObjectLiteral), "jsii-calc.MutableObjectLiteral")]
6+
public interface IMutableObjectLiteral
7+
{
8+
[JsiiProperty("value", "{\"primitive\":\"string\"}")]
9+
string Value
10+
{
11+
get;
12+
set;
13+
}
14+
}
15+
}

‎packages/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/MutableObjectLiteral.cs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
using Amazon.JSII.Runtime.Deputy;
2+
3+
namespace Amazon.JSII.Tests.CalculatorNamespace
4+
{
5+
public class MutableObjectLiteral : DeputyBase, IMutableObjectLiteral
6+
{
7+
[JsiiProperty("value", "{\"primitive\":\"string\"}", true)]
8+
public string Value
9+
{
10+
get;
11+
set;
12+
}
13+
}
14+
}

0 commit comments

Comments
 (0)