HTMLSelectElement won't work with Object.defineProperty: TypeError: 'set' on proxy: trap returned falsish for property <name>

Basic info:

Node.js version: 18.12.0
jsdom version: latest (22.1.0)

Minimal reproduction case

'use strict';

const { JSDOM } = require('jsdom');

const dom = new JSDOM('<select />', {});

const select = dom.window.document.querySelectorAll('select')[0];

const prop = 'test123';

const setter = (v) => {
    console.log('HERE:', v);
    return true;
};

Object.defineProperty(select, prop, {
    set: setter,
});

select.test123 = 123;

// test.js:20
// select.test123 = 123;
//                ^

// TypeError: 'set' on proxy: trap returned falsish for property 'test123'
//     at Object.<anonymous> (/Users/patrickpissurno/Desktop/bug/test.js:20:16)
//     at Module._compile (node:internal/modules/cjs/loader:1159:14)
//     at Module._extensions..js (node:internal/modules/cjs/loader:1213:10)
//     at Module.load (node:internal/modules/cjs/loader:1037:32)
//     at Module._load (node:internal/modules/cjs/loader:878:12)
//     at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
//     at node:internal/main/run_main_module:23:47

How does similar code behave in browsers?

The setter is called and no errors are returned. See here:

<!DOCTYPE html>
<html>
<body>
  <select/>

  <script>
    'use strict';
    
    const select = window.document.querySelectorAll('select')[0];

    const prop = 'test123';

    const setter = (v) => {
        console.log('HERE:', v);
        return true;
    };

    Object.defineProperty(select, prop, {
        set: setter,
    });

    select.test123 = 123;

    // "HERE:"
    // 123
  </script>
</body>
</html>

I've traced the issue to be in the node_modules/jsdom/lib/jsdom/living/generated/HTMLSelectElement.js file, inside of its ProxyHandler class.

There's a piece of code in the ProxyHandler.set(target, P, V, receiver) implementation that does this:

if (!ownDesc.writable) {
  return false;
}

If I change that to:

if (!ownDesc.writable) {
  return Reflect.set(target, P, V, receiver);
}

Everything works as expected.

I honestly don't understand enough to submit a PR, or to even assess whether this is an acceptable fix, but at least it makes sense to me that we should call Reflect.set here instead of assuming we should always return false.

By the way, as far as I can tell this only happens with the select element. The input element, for instance, works just fine, probably because it's not behind a similar Proxy.

I'm pretty sure this is a Node 18 bug in the VM module based on similar issues I've seen previously. Can you try Node 20 and report back?

Edit: hmm nevermind, I think I skimmed too hard. I'll investigate further later.

This is a bug in the following code: https://github.com/jsdom/webidl2js/blob/da99c317dbcadee4f4814daf43c3421be0cf717d/lib/constructs/interface.js#L990-L1016

That code is attempting to implement https://tc39.es/ecma262/multipage/ordinary-and-exotic-objects-behaviours.html#sec-ordinarysetwithowndescriptor . But it seems to be missing the "If IsDataDescriptor(ownDesc) is true, then" precondition. (And it has some small other deviations toward the end, which maybe are observably equivalent; I'm not sure.)

We should probably also pull that out into a helper function so it's not duplicated everywhere.

Fixing this will be a bit tricky as it's coordinated across multiple repositories. Probably it's best for a maintainer to do the full process, but the following are ways you can contribute:

Write a web platform test by finding the appropriate place in the to-upstream directory and following nearby examples. See CONTRIBUTING.md for more. An open pull request with a test that fails on the master branch would be great.
Clone jsdom, yarn install, and then hack on your local copy of webidl2js (inside node_modules) to try to fix this issue. Ideally by implementing the linked algorithm from the ES spec more closely. You can then see if it works by running yarn pretest to regenerate all the generated files (like HTMLSelectElement.js). If it works, either send a PR to webidl2js, or just post the result to this thread, so that it gives the maintainers a starting point when we have time to fix this in earnest.

Okay, so I went with the second approach (hacking on my local copy), and this is what I ended up with, which implements the linked spec algorithm and fixes the issue:

  if (ownDesc === undefined) {
    ownDesc = Reflect.getOwnPropertyDescriptor(target, P);
  }
  if (ownDesc === undefined) {
    const parent = Reflect.getPrototypeOf(target);
    if (parent !== null) {
      return Reflect.set(parent, P, V, receiver);
    }
    ownDesc = { writable: true, enumerable: true, configurable: true, value: undefined };
  }
  const isDataDesc = ownDesc !== undefined && ("value" in ownDesc || "writable" in ownDesc); // added this line
  if(isDataDesc){  // added just the if statement
    if (!ownDesc.writable) {
      return false;
    }
    if (!utils.isObject(receiver)) {
      return false;
    }
    const existingDesc = Reflect.getOwnPropertyDescriptor(receiver, P);
    let valueDesc;
    if (existingDesc !== undefined) {
      if (existingDesc.get || existingDesc.set) {
        return false;
      }
      if (!existingDesc.writable) {
        return false;
      }
      valueDesc = { value: V };
    } else {
      valueDesc = { writable: true, enumerable: true, configurable: true, value: V };
    }
    return Reflect.defineProperty(receiver, P, valueDesc);
  } else {  // added this whole block
    if(ownDesc.set === undefined){
      return false;
    }
    ownDesc.set(V);
    return true;
  }
}

By the way, the extra closing bracket (}) was already there. Hope it helps!

Fix generated set trap for proxy cases

Roll webidl2js and add a test for the defineProperty bug

Fix generated set trap for proxy cases

Navigation Menu

Repository navigation

HTMLSelectElement won't work with Object.defineProperty: TypeError: 'set' on proxy: trap returned falsish for property <name> #3565

Basic info:

Minimal reproduction case

How does similar code behave in browsers?

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

HTMLSelectElement won't work with Object.defineProperty: TypeError: 'set' on proxy: trap returned falsish for property <name> #3565

Description

Basic info:

Minimal reproduction case

How does similar code behave in browsers?

Activity

domenic commented on Jun 8, 2023

domenic commented on Jun 8, 2023

patrickpissurno commented on Jun 8, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions