Skip to content
New issue

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

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

Already on GitHub? # to your account

Non-registered symbols are treated as invalid weak map key #49135

Closed
TaciturnCoder opened this issue Aug 13, 2023 · 2 comments
Closed

Non-registered symbols are treated as invalid weak map key #49135

TaciturnCoder opened this issue Aug 13, 2023 · 2 comments

Comments

@TaciturnCoder
Copy link

Version

v19.7.0

Platform

Microsoft Windows NT 10.0.19045.0 x64

Subsystem

No response

What steps will reproduce the bug?

/********************************************************************
 * Reference-Mapped data structure
 */
export class RefMap {
  #map;

  constructor() {
    this.#map = new WeakMap();
  }

  set(node) {
    const ref = Symbol();
    this.#map.set(ref, node); // Error !!!
    return ref;
  }

  has(ref) { return this.#map.has(ref); }
  get(ref) { return this.#map.get(ref); }
  delete(ref) { return this.#map.delete(ref); }
}


// Test.mjs
// import { RefMap } from "./refmap.mjs";

let secret = null;
const messages = new RefMap();

// Create a secret message channel
secret = messages.set([]); // Error !!!

// Add messages
messages.get(secret).push("Hello World!");

// Delete secret, messages get garbage collected.
secret = null;

How often does it reproduce? Is there a required condition?

No response

What is the expected behavior? Why is that the expected behavior?

Refering to MDN documentation:

A WeakMap is a collection of key/value pairs whose keys must be objects or non-registered symbols, with values of any arbitrary JavaScript type, and which does not create strong references to its keys.

Emphasis on non-registered symbols. NodeJS gives TypeError: Invalid value used as weak map key when non-registered symbols are used as keys for WeakMap entries.

What do you see instead?

file:///*:/************/****/******/refmap.mjs:13
    this.#map.set(ref, node);
              ^

TypeError: Invalid value used as weak map key
    at WeakMap.set (<anonymous>)
    at RefMap.set (file:///*:/************/****/******/refmap.mjs:13:15)
    at file:///*:/************/****/******/test.mjs:7:19
    at ModuleJob.run (node:internal/modules/esm/module_job:193:25)        

Node.js v19.7.0

Additional information

Works without error on web browsers (tested on Chrome, Edge)

@atlowChemi
Copy link
Member

atlowChemi commented Aug 13, 2023

This is not working on v19.7 since it is a (rather) new addition to TC39: https://github.com/tc39/proposal-symbols-as-weakmap-keys

Node v19.7 uses v8 10.8.168.25-node.11 whereas v20.1.0 was upgraded in (13105c1) to use 11.3.244.8-node.10, hence this script runs fine on v20.5.0 🙂

Also worth noting that v19.x is EOL and you might want to consider upgrading to v20: https://github.com/nodejs/release

@TaciturnCoder
Copy link
Author

👍 I upgraded to NodeJS v20.5.1 and confirm the script is working as intended. Marking this issue as closed.

miyasudokoro added a commit to miyasudokoro/browser-compat-data that referenced this issue Feb 19, 2024
According to nodejs/node#49135 this support was added in 20.1.0.
Elchi3 added a commit to mdn/browser-compat-data that referenced this issue Feb 20, 2024
* Update WeakMap.json NodeJS symbol_as_keys

According to nodejs/node#49135 this support was added in 20.1.0.

* Update WeakMap.json

use the first version of Node that's in the list

* Update nodejs.json

Add 20.1.0

* Use 20.1.0

---------

Co-authored-by: Florian Scholz <fs@florianscholz.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants