Skip to content

Commit eb142d8

Browse files
committed
Return proxy from constructor further up the prototype chain
The change ensures the this object used in methods is the proxy from the very beginning. This solves pretty much all issues with returning the proxy from a constructor. For example, private fields can be used now. More details available at: tj#1921 (comment) Additionally, wrong spelling has been fixed in comments.
1 parent a3f0e28 commit eb142d8

File tree

1 file changed

+69
-62
lines changed

1 file changed

+69
-62
lines changed

lib/command.js

Lines changed: 69 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,69 @@ const { suggestSimilar } = require('./suggestSimilar');
1212

1313
// @ts-check
1414

15-
class Command extends EventEmitter {
15+
class CommandBase extends EventEmitter {
16+
constructor() {
17+
super();
18+
19+
// The proxy only treats keys not present in the instance and its prototype chain as keys for _optionValues when _storeOptionsAsProperties is set to true.
20+
// Setting option values for keys present in the instance and its prototype chain is still possible by calling .setOptionValue() or .setOptionValueWithSource(),
21+
// but such values will not be accessible as instance properties because the instance and its prototype chain have precedence.
22+
// However, they will be accessible via .getOptionValue(), .opts() and .optsWithGlobals().
23+
return new Proxy(this, {
24+
get(target, key, receiver) {
25+
if (target._storeOptionsAsProperties && !(key in target)) {
26+
target = receiver = receiver._optionValuesProxy;
27+
}
28+
return Reflect.get(target, key, receiver);
29+
},
30+
set(target, key, value, receiver) {
31+
if (target._storeOptionsAsProperties && !(key in target)) {
32+
target = receiver = receiver._optionValuesProxy;
33+
}
34+
return Reflect.set(target, key, value, receiver);
35+
},
36+
has(target, key) {
37+
if (target._storeOptionsAsProperties && !(key in target)) {
38+
target = target._optionValuesProxy;
39+
}
40+
return Reflect.has(target, key);
41+
},
42+
deleteProperty(target, key) {
43+
if (target._storeOptionsAsProperties && !(key in target)) {
44+
target = target._optionValuesProxy;
45+
}
46+
return Reflect.deleteProperty(target, key);
47+
},
48+
defineProperty(target, key, descriptor) {
49+
if (target._storeOptionsAsProperties && !(key in target)) {
50+
target = target._optionValuesProxy;
51+
}
52+
return Reflect.defineProperty(target, key, descriptor);
53+
},
54+
getOwnPropertyDescriptor(target, key) {
55+
if (target._storeOptionsAsProperties && !(key in target)) {
56+
target = target._optionValuesProxy;
57+
}
58+
return Reflect.getOwnPropertyDescriptor(target, key);
59+
},
60+
ownKeys(target) {
61+
const result = Reflect.ownKeys(target);
62+
if (target._storeOptionsAsProperties) {
63+
result.push(...Reflect.ownKeys(target._optionValuesProxy));
64+
}
65+
return result;
66+
},
67+
preventExtensions(target) {
68+
if (target._storeOptionsAsProperties) {
69+
Reflect.preventExtensions(target._optionValuesProxy);
70+
}
71+
return Reflect.preventExtensions(target);
72+
}
73+
});
74+
}
75+
}
76+
77+
class Command extends CommandBase {
1678
/**
1779
* Initialize a new `Command`.
1880
*
@@ -78,6 +140,12 @@ class Command extends EventEmitter {
78140
this._helpCommandDescription = 'display help for command';
79141
this._helpConfiguration = {};
80142

143+
// Because of how the proxy returned from the CommandBase constructor works in order to support options-as-properties,
144+
// all instance properties have to be defined when _storeOptionsAsProperties is set to false.
145+
// Ideally, that should happen as soon as in the constructor, even if it seems unnecessary because the initial values are undefined like here.
146+
this._version = undefined;
147+
this._versionOptionName = undefined;
148+
81149
// Double proxy to show the version option property value instead of [Getter/Setter] when printing the return value of opts() to a console.
82150
// Required because Node internally unwraps one proxy and therefore would not use the getOwnPropertyDescriptor() trap otherwise.
83151
this._optionValuesProxy = new Proxy(new Proxy(this._optionValues, {
@@ -111,67 +179,6 @@ Options value configuration is not supported`);
111179
return Reflect.getOwnPropertyDescriptor(target, key);
112180
}
113181
}), {});
114-
115-
// Because of how the returned proxy works, ideally, no prooerties should be defined outside the cinstructor.
116-
// They can still be defined outside the constructor in subclasses, but only when _storeOptionsAsProperties is set to false.
117-
this._version = undefined;
118-
this._versionOptionName = undefined;
119-
120-
// The proxy only treats keys not present in the instance and its prototype chain as keys for _optionValues when _storeOptionsAsProperties is set to true.
121-
// Setting option values for keys present in the instance and its prototype chain is still possible by calling .setOptionValue() or .setOptionValueWithSource(),
122-
// but such values will not be accessible as instnace properties because the instance and its prototype chain has precedence.
123-
// However, they will be accessible via .getOptionValue(), .opts() and .optsWithGlobals().
124-
return new Proxy(this, {
125-
get(target, key, receiver) {
126-
if (target._storeOptionsAsProperties && !(key in target)) {
127-
target = receiver = receiver._optionValuesProxy;
128-
}
129-
return Reflect.get(target, key, receiver);
130-
},
131-
set(target, key, value, receiver) {
132-
if (target._storeOptionsAsProperties && !(key in target)) {
133-
target = receiver = receiver._optionValuesProxy;
134-
}
135-
return Reflect.set(target, key, value, receiver);
136-
},
137-
has(target, key) {
138-
if (target._storeOptionsAsProperties && !(key in target)) {
139-
target = target._optionValuesProxy;
140-
}
141-
return Reflect.has(target, key);
142-
},
143-
deleteProperty(target, key) {
144-
if (target._storeOptionsAsProperties && !(key in target)) {
145-
target = target._optionValuesProxy;
146-
}
147-
return Reflect.deleteProperty(target, key);
148-
},
149-
defineProperty(target, key, descriptor) {
150-
if (target._storeOptionsAsProperties && !(key in target)) {
151-
target = target._optionValuesProxy;
152-
}
153-
return Reflect.defineProperty(target, key, descriptor);
154-
},
155-
getOwnPropertyDescriptor(target, key) {
156-
if (target._storeOptionsAsProperties && !(key in target)) {
157-
target = target._optionValuesProxy;
158-
}
159-
return Reflect.getOwnPropertyDescriptor(target, key);
160-
},
161-
ownKeys(target) {
162-
const result = Reflect.ownKeys(target);
163-
if (target._storeOptionsAsProperties) {
164-
result.push(...Reflect.ownKeys(target._optionValuesProxy));
165-
}
166-
return result;
167-
},
168-
preventExtensions(target) {
169-
if (target._storeOptionsAsProperties) {
170-
Reflect.preventExtensions(target._optionValuesProxy);
171-
}
172-
return Reflect.preventExtensions(target);
173-
}
174-
});
175182
}
176183

177184
/**

0 commit comments

Comments
 (0)