Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeError: Cannot assign to read only property 'name' of function in lib/sinon/util/core/extend.js #2203

Closed
jandockx opened this issue Jan 27, 2020 · 2 comments

Comments

@jandockx
Copy link

Describe the bug

When creating a spy for a function that has an explicit name property, lib/sinon/util/core/extend.js fails on line 88:

TypeError: Cannot assign to read only property 'name' of function 'function proxy() {
                return p.invoke(func, this, slice(arguments));
            }'
    at copyValue (node_modules/sinon/lib/sinon/util/core/extend.js:88:20)
    at extendCommon (node_modules/sinon/lib/sinon/util/core/extend.js:62:17)
    at extend (node_modules/sinon/lib/sinon/util/core/extend.js:87:12)
    at createProxy (node_modules/sinon/lib/sinon/proxy.js:196:5)
    at createSpy (node_modules/sinon/lib/sinon/spy.js:122:17)
    at Function.spy (node_modules/sinon/lib/sinon/spy.js:157:45)
    at Sandbox.spy (node_modules/sinon/lib/sinon/sandbox.js:322:35)
    at Context.<anonymous> (test/FlowLogger.test.js:168:44)
    at processImmediate (internal/timers.js:439:21)

The reason is that proxy.name is not writeable.

To Reproduce

Try to spy on a function that has a name as an own property.

  it('cannot spy', function () {
    const brandNewName = 'my brand new name'
    function importantFunction () {}

    importantFunction.name.should.equal('importantFunction')

    Object.defineProperty(importantFunction, 'name', {
      configurable: false,
      enumerable: true,
      value: brandNewName,
      writable: false
    })

    console.log(importantFunction)
    importantFunction.name.should.equal(brandNewName)

    function callSomethingImportant () {
      importantFunction()
    }

    const sandbox = sinon.createSandbox()
    const stub = sandbox.spy(importantFunction) // fails

    callSomethingImportant()
    stub.should.be.calledOnce()
  })

Expected behavior

Make the proxy have the same name, or skip copying the name to the proxy.

Solution

I believe you can do this, by not copying with dest[prop] = source[prop], but by defining the property on dest as derived:

    return extendCommon(target, sources, function copyValue(dest, source, prop) {
        const sourcePropDescriptor = Object.getOwnPropertyDescriptor(source, prop)
        Object.defineProperty(dest, prop, {
            configurable: sourcePropDescriptor.configurable,
            enumerable: true, // otherwise we would not try to do this: we are iterating over enumerable own properties
            writable: sourcePropDescriptor.writable,
            value: source[prop]
        })
    });

If that's not what you want to do, you can skip copying properties that are not writeable on the target function. name is the only one I can come up with for a function, so, since this is a border case, that might be good enough (but less ideal):

    return extendCommon(target, sources, function copyValue(dest, source, prop) {
        if (prop !== 'name') {
            dest[prop] = source[prop];
        }
    });

Or, combine both approaches, for performance reasons:

    return extendCommon(target, sources, function copyValue(dest, source, prop) {
        if (prop !== 'name') {
            const sourcePropDescriptor = Object.getOwnPropertyDescriptor(source, prop)
            Object.defineProperty(dest, prop, {
                configurable: sourcePropDescriptor.configurable,
                enumerable: true, // otherwise we would not try to do this: we are iterating over enumerable own properties
                writable: sourcePropDescriptor.writable,
                value: source[prop]
            })
        }
        else {
            dest[prop] = source[prop];
        }
    });

Context (please complete the following information):

  • Library version: 8.1.1
  • Environment: Node 12.14.0
  • Example URL: N/A
  • Other libraries you are using: @toryt/contracts-iv

Additional context
N/A

@stale
Copy link

stale bot commented Mar 27, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 27, 2020
@jandockx
Copy link
Author

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants