src,lib,test: unflag --experimental-webstorage by default

[danielmbrasil]

Added Web Storage support enabled by default, with the option to disable via --no-webstorage.

[nodejs-github-bot]

Review requested:

• @nodejs/config

[mcollina]

To fix the failing test, I would change the error being throw in lib/internal/webstorage.js in a warning, and have localStorage be undefined in that case.

@cjihrig wdyt?

[cjihrig]

I would be OK with that behavior. However, the global checking logic in test/common/index.js would probably need to be updated. Otherwise, the new warning will probably be printed for every test that doesn't use localStorage.

[danielmbrasil]

I just pushed the suggested changes, and the tests are now passing. @cjihrig, could you please review whether the changes in test/common/index.js make sense?

[cjihrig]

Without running the code, it doesn't look correct to me:

globalThis[val] === undefined

By the time we find out that the value is undefined, the warning will have already been emitted, right?

[danielmbrasil]

By the time we find out that the value is undefined, the warning will have already been emitted, right?

Yeah, that's right. I'll take a look on how to avoid the warning to be emitted in those cases.

[danielmbrasil]

@cjihrig Could you take a look now and see if this is closer to what you were expecting? I added a new check to test/common/index.js that overrides the localStorage getter with an empty object. This prevents the warning from being emitted, and the tests are passing locally.

[danielmbrasil]

Moving this as ready for review. I think we should possibly also update doc/api/cli.md and doc/api/globals.md, but I'm not sure how to update those docs for now.

[codecov]

Codecov Report

❌ Patch coverage is 82.75862% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.28%. Comparing base (73b5022) to head (890a794).
⚠️ Report is 116 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/webstorage.js	80.00%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #57666      +/-   ##
==========================================
+ Coverage   88.26%   88.28%   +0.02%     
==========================================
  Files         701      701              
  Lines      206644   206676      +32     
  Branches    39737    39738       +1     
==========================================
+ Hits       182386   182472      +86     
+ Misses      16279    16237      -42     
+ Partials     7979     7967      -12     

Files with missing lines	Coverage Δ
lib/internal/process/pre_execution.js	97.15% <100.00%> (-0.28%)	⬇️
src/node_options.cc	77.89% <100.00%> (+0.02%)	⬆️
src/node_options.h	97.89% <100.00%> (ø)
lib/internal/webstorage.js	91.30% <80.00%> (-8.70%)	⬇️

... and 41 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[mcollina]

A few tests needs fixing but this is good progress!

[geeksilva97]

This PR has some conflicts

[mcollina]

lgtm

[geeksilva97]

Appreciate your effort here, Daniel

[mcollina]

lgtm

[mcollina]

@jasnell PTAL

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/69147/

[nodejs-github-bot]

Landed in 3312e4e

[github-actions]

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @mcollina.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.