Skip to content

Conversation

@islandryu
Copy link
Member

As shown in the image, this change is intended to enable inspecting storage in DevTools.

Screenshot 2025-12-21 at 22 24 53

Currently, there is no feature to automatically record storage. Instead, storage data is registered using the DOMStorage.registerStorage method.

DOMStorage.registerStorage({
  isLocalStorage,
  storageMap: {
    key1: 'value1',
    key2: 'value2',
    [1]: 2,
    [true]: 'booleanKey',
  },
});

If this change is acceptable, I am planning to submit the following changes to Chrome DevTools.

https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7274801

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/config
  • @nodejs/gyp
  • @nodejs/inspector

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Dec 21, 2025
@islandryu islandryu force-pushed the feat/storageInspection branch from cb8ff6d to 2150295 Compare December 21, 2025 14:10
@islandryu islandryu force-pushed the feat/storageInspection branch from 2150295 to 611b71e Compare December 21, 2025 14:17
@codecov
Copy link

codecov bot commented Dec 21, 2025

Codecov Report

❌ Patch coverage is 63.75000% with 87 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.50%. Comparing base (1cd3ddf) to head (19be6a8).
⚠️ Report is 208 commits behind head on main.

Files with missing lines Patch % Lines
src/inspector/dom_storage_agent.cc 50.70% 57 Missing and 13 partials ⚠️
src/inspector/inspector_object_utils.cc 77.55% 3 Missing and 8 partials ⚠️
src/inspector/storage_agent.cc 70.00% 3 Missing ⚠️
src/inspector/notification_emitter.cc 71.42% 0 Missing and 2 partials ⚠️
src/inspector_agent.cc 95.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #61139      +/-   ##
==========================================
- Coverage   88.53%   88.50%   -0.04%     
==========================================
  Files         703      708       +5     
  Lines      208260   208778     +518     
  Branches    40160    40268     +108     
==========================================
+ Hits       184384   184774     +390     
- Misses      15882    16012     +130     
+ Partials     7994     7992       -2     
Files with missing lines Coverage Δ
lib/inspector.js 97.22% <100.00%> (+0.12%) ⬆️
src/inspector/network_agent.cc 61.67% <ø> (-1.81%) ⬇️
src/inspector/notification_emitter.h 100.00% <100.00%> (ø)
src/node_options.cc 77.82% <ø> (-0.04%) ⬇️
src/node_options.h 97.89% <ø> (+0.03%) ⬆️
src/inspector_agent.cc 80.65% <95.00%> (+0.70%) ⬆️
src/inspector/notification_emitter.cc 71.42% <71.42%> (ø)
src/inspector/storage_agent.cc 70.00% <70.00%> (ø)
src/inspector/inspector_object_utils.cc 77.55% <77.55%> (ø)
src/inspector/dom_storage_agent.cc 50.70% <50.70%> (ø)

... and 89 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.

@islandryu islandryu force-pushed the feat/storageInspection branch from 40b69c5 to 4d0900b Compare December 22, 2025 14:48
@legendecas legendecas added the inspector Issues and PRs related to the V8 inspector protocol label Dec 22, 2025
Copy link
Member

@legendecas legendecas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder how the Node.js APIs like node:sqlite would be represented on the devtools frontend. Showing it as either local storage or session sotrage would be confusing IMO.

/cc @nodejs/sqlite for opinion.

@louwers
Copy link
Contributor

louwers commented Dec 22, 2025

@legendecas It would not make sense to include it, because unlike local storage or session storage, SQLite databases need to be explicitly opened. There is no global SQLite database.

@legendecas
Copy link
Member

legendecas commented Dec 22, 2025

So this feature should only be enabled for localStorage as far as I could tell.

The local storage file path is set via cli --localstorage-file=file so I think the origin of the storage could be pointed to a file URL to this file path.

islandryu and others added 2 commits December 23, 2025 09:44
Co-authored-by: Chengzhong Wu <legendecas@gmail.com>
@islandryu
Copy link
Member Author

I agree with limiting this implementation to localStorage.

If we want to support other storage types in the future, I think we should either use the existing IndexedDB DevTools domain
https://chromedevtools.github.io/devtools-protocol/tot/IndexedDB/

or add a dedicated section in DevTools for them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants