| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -731,35 +731,15 @@ function includesProxiesOrGetters(expr, exprStr, evalFn, ctx, callback) { | |||
| 731 | 731 | ||
| 732 | 732 | if (astProp.type === 'Literal') { | |
| 733 | 733 | // We have something like `obj['foo'].x` where `x` is the literal | |
| 734 | - | ||
| 735 | - if (safeIsProxyAccess(obj, astProp.value)) { | ||
| 736 | - return cb(true); | ||
| 737 | - } | ||
| 738 | - | ||
| 739 | - const propDescriptor = ObjectGetOwnPropertyDescriptor( | ||
| 740 | - obj, | ||
| 741 | - `${astProp.value}`, | ||
| 742 | - ); | ||
| 743 | - const propHasGetter = typeof propDescriptor?.get === 'function'; | ||
| 744 | - return cb(propHasGetter); | ||
| 734 | + return propHasGetterOrIsProxy(obj, astProp.value, cb); | ||
| 745 | 735 | } | |
| 746 | 736 | ||
| 747 | 737 | if ( | |
| 748 | 738 | astProp.type === 'Identifier' && | |
| 749 | 739 | exprStr.at(astProp.start - 1) === '.' | |
| 750 | 740 | ) { | |
| 751 | 741 | // We have something like `obj.foo.x` where `foo` is the identifier | |
| 752 | - | ||
| 753 | - if (safeIsProxyAccess(obj, astProp.name)) { | ||
| 754 | - return cb(true); | ||
| 755 | - } | ||
| 756 | - | ||
| 757 | - const propDescriptor = ObjectGetOwnPropertyDescriptor( | ||
| 758 | - obj, | ||
| 759 | - `${astProp.name}`, | ||
| 760 | - ); | ||
| 761 | - const propHasGetter = typeof propDescriptor?.get === 'function'; | ||
| 762 | - return cb(propHasGetter); | ||
| 742 | + return propHasGetterOrIsProxy(obj, astProp.name, cb); | ||
| 763 | 743 | } | |
| 764 | 744 | ||
| 765 | 745 | return evalFn( | |
@@ -773,37 +753,48 @@ function includesProxiesOrGetters(expr, exprStr, evalFn, ctx, callback) { | |||
| 773 | 753 | getREPLResourceName(), | |
| 774 | 754 | (err, evaledProp) => { | |
| 775 | 755 | if (err) { | |
| 776 | - return callback(false); | ||
| 756 | + return cb(false); | ||
| 777 | 757 | } | |
| 778 | 758 | ||
| 779 | 759 | if (typeof evaledProp === 'string') { | |
| 780 | - if (safeIsProxyAccess(obj, evaledProp)) { | ||
| 781 | - return cb(true); | ||
| 782 | - } | ||
| 783 | - | ||
| 784 | - const propDescriptor = ObjectGetOwnPropertyDescriptor( | ||
| 785 | - obj, | ||
| 786 | - evaledProp, | ||
| 787 | - ); | ||
| 788 | - const propHasGetter = typeof propDescriptor?.get === 'function'; | ||
| 789 | - return cb(propHasGetter); | ||
| 760 | + return propHasGetterOrIsProxy(obj, evaledProp, cb); | ||
| 790 | 761 | } | |
| 791 | 762 | ||
| 792 | - return callback(false); | ||
| 763 | + return cb(false); | ||
| 793 | 764 | }, | |
| 794 | 765 | ); | |
| 795 | 766 | } | |
| 796 | 767 | ||
| 797 | - function safeIsProxyAccess(obj, prop) { | ||
| 798 | - // Accessing `prop` may trigger a getter that throws, so we use try-catch to guard against it | ||
| 799 | - try { | ||
| 800 | - return isProxy(obj[prop]); | ||
| 801 | - } catch { | ||
| 802 | - return false; | ||
| 803 | - } | ||
| 768 | + return callback(false); | ||
| 769 | + } | ||
| 770 | + | ||
| 771 | + /** | ||
| 772 | + * Given an object and a property name, checks whether the property has a getter, if not checks whether its | ||
| 773 | + * value is a proxy. | ||
| 774 | + * | ||
| 775 | + * Note: the order is relevant here, we want to check whether the property has a getter _before_ we check | ||
| 776 | + * whether its value is a proxy, to ensure that is the property does have a getter we don't end up | ||
| 777 | + * triggering it when checking its value | ||
| 778 | + * @param {any} obj The target object | ||
| 779 | + * @param {string | number | bigint | boolean | RegExp} prop The target property | ||
| 780 | + * @param {(includes: boolean) => void} cb Callback that will be called with the result of the operation | ||
| 781 | + * @returns {void} | ||
| 782 | + */ | ||
| 783 | + function propHasGetterOrIsProxy(obj, prop, cb) { | ||
| 784 | + const propDescriptor = ObjectGetOwnPropertyDescriptor( | ||
| 785 | + obj, | ||
| 786 | + prop, | ||
| 787 | + ); | ||
| 788 | + const propHasGetter = typeof propDescriptor?.get === 'function'; | ||
| 789 | + if (propHasGetter) { | ||
| 790 | + return cb(true); | ||
| 804 | 791 | } | |
| 805 | 792 | ||
| 806 | - return callback(false); | ||
| 793 | + if (isProxy(obj[prop])) { | ||
| 794 | + return cb(true); | ||
| 795 | + } | ||
| 796 | + | ||
| 797 | + return cb(false); | ||
| 807 | 798 | } | |
| 808 | 799 | ||
| 809 | 800 | module.exports = { | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -115,6 +115,33 @@ describe('REPL completion in relation of getters', () => { | |||
| 115 | 115 | ['objWithGetters[getGFooKey()].b', []], | |
| 116 | 116 | ]); | |
| 117 | 117 | }); | |
| 118 | + | ||
| 119 | + test('no side effects are triggered for getters during completion', async () => { | ||
| 120 | + const { replServer } = startNewREPLServer(); | ||
| 121 | + | ||
| 122 | + await new Promise((resolve, reject) => { | ||
| 123 | + replServer.eval('const foo = { get name() { globalThis.nameGetterRun = true; throw new Error(); } };', | ||
| 124 | + replServer.context, '', (err) => { | ||
| 125 | + if (err) { | ||
| 126 | + reject(err); | ||
| 127 | + } else { | ||
| 128 | + resolve(); | ||
| 129 | + } | ||
| 130 | + }); | ||
| 131 | + }); | ||
| 132 | + | ||
| 133 | + ['foo.name.', 'foo["name"].'].forEach((test) => { | ||
| 134 | + replServer.complete( | ||
| 135 | + test, | ||
| 136 | + common.mustCall((error, data) => { | ||
| 137 | + // The context's nameGetterRun variable hasn't been set | ||
| 138 | + assert.strictEqual(replServer.context.nameGetterRun, undefined); | ||
| 139 | + // No errors has been thrown | ||
| 140 | + assert.strictEqual(error, null); | ||
| 141 | + }) | ||
| 142 | + ); | ||
| 143 | + }); | ||
| 144 | + }); | ||
| 118 | 145 | }); | |
| 119 | 146 | ||
| 120 | 147 | describe('completions on proxies', () => { | |
| Back | FazBrowse Home | New Git URL |
0 commit comments