Skip to content

Commit

Permalink
fix(client): Force result extensions to log correct properties (#16542
Browse files Browse the repository at this point in the history
)

Node.js `util.inspect` and console functions deliberately avoid
triggering any of the proxy traps. If result extensions are used, that
forces us to show completely incorrect result - it will show the target
which would not match what user is actually getting there.
  • Loading branch information
SevInf authored and jkomyno committed Dec 21, 2022
1 parent 5b95f3c commit 362cf55
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import util from 'util'

import { createCompositeProxy } from './createCompositeProxy'

test('forwards properties to the target', () => {
Expand Down Expand Up @@ -52,6 +54,22 @@ test('allows to add multiple properties via single layer', () => {
expect(proxy).toHaveProperty('second', 2)
})

test('logs proxy properties if used in console.log/util.inspect', () => {
const proxy = createCompositeProxy({}, [
{
getKeys() {
return ['first', 'second']
},

getPropertyValue(key) {
return key === 'first' ? 1 : 2
},
},
])

expect(util.inspect(proxy)).toMatchInlineSnapshot(`{ first: 1, second: 2 }`)
})

test('allows to set property descriptor via layer', () => {
const proxy = createCompositeProxy({}, [
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { type InspectOptions, inspect } from 'util'

import { defaultPropertyDescriptor } from '../model/utils/defaultProxyHandlers'

export interface CompositeProxyLayer<KeyType extends string | symbol = string | symbol> {
Expand Down Expand Up @@ -27,6 +29,8 @@ export interface CompositeProxyLayer<KeyType extends string | symbol = string |
has?(key: KeyType): boolean
}

const customInspect = Symbol.for('nodejs.util.inspect.custom')

/**
* Creates a proxy from a set of layers.
* Each layer is a building for a proxy (potentially, reusable) that
Expand All @@ -41,7 +45,7 @@ export function createCompositeProxy<T extends object>(target: T, layers: Compos
const keysToLayerMap = mapKeysToLayers(layers)
const overwrittenKeys = new Set<string | symbol>()

return new Proxy(target, {
const proxy = new Proxy(target, {
get(target, prop) {
// explicit overwrites of a property have highest priority
if (overwrittenKeys.has(prop)) {
Expand Down Expand Up @@ -96,6 +100,16 @@ export function createCompositeProxy<T extends object>(target: T, layers: Compos
return defaultPropertyDescriptor
},
})

proxy[customInspect] = function (depth: number, options: InspectOptions, defaultInspect: typeof inspect = inspect) {
// Default node.js console.log and util.inspect deliberately avoid triggering any proxy traps and log
// original target. This is not we want for our usecases: we want console.log to output the result as if
// the properties actually existed on the target. Using spread operator forces us to produce correct object
const toLog = { ...this }
delete toLog[customInspect]
return defaultInspect(toLog, options)
}
return proxy
}

function mapKeysToLayers(layers: CompositeProxyLayer[]) {
Expand Down

0 comments on commit 362cf55

Please sign in to comment.