Unflag --experimental-webstorage

[mcollina]

I think we could start shipping with localStorage enabled by default in v25.

[mcollina]

cc @cjihrig

[cjihrig]

SGTM

[danielmbrasil]

Hey, could you give some guidance on what needs to be done to unflag --experimental-webstorage? I tried setting this option as the default in node_options.cc and node_options.h as shown in the diff:

diff --git a/src/node_options.cc b/src/node_options.cc
index 16cce0df0e..d30b0b87b6 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -456,7 +456,8 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
   AddOption("--experimental-webstorage",
             "experimental Web Storage API",
             &EnvironmentOptions::experimental_webstorage,
-            kAllowedInEnvvar);
+            kAllowedInEnvvar,
+            true);
   AddOption("--localstorage-file",
             "file used to persist localStorage data",
             &EnvironmentOptions::localstorage_file,
diff --git a/src/node_options.h b/src/node_options.h
index baa615e310..07cda97d71 100644
--- a/src/node_options.h
+++ b/src/node_options.h
@@ -126,7 +126,7 @@ class EnvironmentOptions : public Options {
   bool experimental_fetch = true;
   bool experimental_websocket = true;
   bool experimental_sqlite = true;
-  bool experimental_webstorage = false;
+  bool experimental_webstorage = true;
 #ifdef NODE_OPENSSL_HAS_QUIC
   bool experimental_quic = false;
 #endif

This change works when running node directly:

But the tests in test/parallel/test-webstorage.js are failing and I'm not sure why. I'd appreciate it if you could give me a hint on what I'm missing, thanks!

EDIT:

The tests fail with the following error:

node:internal/webstorage:30
          throw new ERR_INVALID_ARG_VALUE('--localstorage-file',
          ^

TypeError [ERR_INVALID_ARG_VALUE]: The argument '--localstorage-file' is an invalid localStorage location. Received ''
    at Object.get [as localStorage] (node:internal/webstorage:30:17)
    at get localStorage (node:internal/util:629:20)
    at /home/daniel/open_source/node/test/common/index.js:318:17
    at Array.forEach (<anonymous>)
    at Object.<anonymous> (/home/daniel/open_source/node/test/common/index.js:317:3)
    at Module._compile (node:internal/modules/cjs/loader:1734:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1899:10)
    at Module.load (node:internal/modules/cjs/loader:1469:32)
    at Module._load (node:internal/modules/cjs/loader:1286:12)
    at TracingChannel.traceSync (node:diagnostics_channel:322:14) {
  code: 'ERR_INVALID_ARG_VALUE'
}

Node.js v24.0.0-pre

[mcollina]

Probably there is something else to fix.

[mcollina]

Can you push what you have in a draft PR?

[geeksilva97]

In test/common/index.js it adds localStorage to the globalThis, but this requires the localStorage file.

seems like it has the same effect as ./node -e 'localStorage'

[danielmbrasil]

Can you push what you have in a draft PR?

@mcollina That's been pushed (#57666)