Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion lib/internal/process/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -404,8 +404,26 @@ function setupWebStorage() {

// https://html.spec.whatwg.org/multipage/webstorage.html#webstorage
exposeLazyInterfaces(globalThis, 'internal/webstorage', ['Storage']);

// localStorage is non-enumerable when --localstorage-file is not provided
// to avoid breaking {...globalThis} operations.
const localStorageFile = getOptionValue('--localstorage-file');
let lazyLocalStorage;
ObjectDefineProperty(globalThis, 'localStorage', {
__proto__: null,
enumerable: localStorageFile !== '',
configurable: true,
get() {
lazyLocalStorage ??= require('internal/webstorage').localStorage;
return lazyLocalStorage;
},
set(value) {
lazyLocalStorage = value;
},
});

defineReplaceableLazyAttribute(globalThis, 'internal/webstorage', [
'localStorage', 'sessionStorage',
Copy link
Member

Choose a reason for hiding this comment

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

Couldn't you keep this call the same, and do something like

ObjectDefineProperty(globalThis, 'localStorage', {
  enumerable: ...
});

Instead of re-writing the whole implementation?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm sorry, but I don't understand what you are asking.

'sessionStorage',
]);
}

Expand Down
27 changes: 16 additions & 11 deletions lib/internal/webstorage.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ const {
ObjectDefineProperties,
} = primordials;
const { getOptionValue } = require('internal/options');
const { lazyDOMException } = require('internal/util');
const { kConstructorKey, Storage } = internalBinding('webstorage');
const { getValidatedPath } = require('internal/fs/utils');
const kInMemoryPath = ':memory:';
Expand All @@ -12,26 +11,32 @@ module.exports = { Storage };

let lazyLocalStorage;
let lazySessionStorage;
let localStorageWarned = false;

// Check at load time if localStorage file is provided to determine enumerability.
// If not provided, localStorage is non-enumerable to avoid breaking {...globalThis}.
const localStorageLocation = getOptionValue('--localstorage-file');

ObjectDefineProperties(module.exports, {
__proto__: null,
localStorage: {
__proto__: null,
configurable: true,
enumerable: true,
enumerable: localStorageLocation !== '',
get() {
if (lazyLocalStorage === undefined) {
// For consistency with the web specification, throw from the accessor
// if the local storage path is not provided.
const location = getOptionValue('--localstorage-file');
if (location === '') {
throw lazyDOMException(
'Cannot initialize local storage without a `--localstorage-file` path',
'SecurityError',
);
if (localStorageLocation === '') {
if (!localStorageWarned) {
localStorageWarned = true;
process.emitWarning(
'localStorage is not available because --localstorage-file was not provided.',
'ExperimentalWarning',
);
}
return undefined;
}

lazyLocalStorage = new Storage(kConstructorKey, getValidatedPath(location));
lazyLocalStorage = new Storage(kConstructorKey, getValidatedPath(localStorageLocation));
}

return lazyLocalStorage;
Expand Down
9 changes: 4 additions & 5 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,10 @@ const hasSQLite = Boolean(process.versions.sqlite);
const hasQuic = hasCrypto && !!process.features.quic;

const hasLocalStorage = (() => {
try {
return hasSQLite && globalThis.localStorage !== undefined;
} catch {
return false;
}
// Check enumerable property to avoid triggering the getter which emits a warning.
// localStorage is enumerable only when --localstorage-file is provided.
const desc = Object.getOwnPropertyDescriptor(globalThis, 'localStorage');
return hasSQLite && desc?.enumerable === true;
})();

/**
Expand Down
7 changes: 6 additions & 1 deletion test/parallel/test-global.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,12 @@ for (const moduleName of builtinModules) {
'navigator',
];
if (common.hasSQLite) {
expected.push('localStorage', 'sessionStorage');
// sessionStorage is always enumerable when SQLite is available.
// localStorage is only enumerable when --localstorage-file is provided.
expected.push('sessionStorage');
if (common.hasLocalStorage) {
expected.push('localStorage');
}
}
assert.deepStrictEqual(new Set(Object.keys(globalThis)), new Set(expected));
expected.forEach((value) => {
Expand Down
17 changes: 13 additions & 4 deletions test/parallel/test-webstorage.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,22 @@ test('sessionStorage is not persisted', async () => {
assert.strictEqual((await readdir(tmpdir.path)).length, 0);
});

test('localStorage throws without --localstorage-file', async () => {
test('localStorage returns undefined and warns without --localstorage-file', async () => {
const cp = await spawnPromisified(process.execPath, [
'-e', 'localStorage',
'-pe', 'localStorage',
]);
assert.strictEqual(cp.code, 1);
assert.strictEqual(cp.code, 0);
assert.strictEqual(cp.signal, null);
assert.match(cp.stderr, /SecurityError:/);
assert.match(cp.stdout, /undefined/);
assert.match(cp.stderr, /ExperimentalWarning:.*localStorage is not available/);
});

test('localStorage is not enumerable without --localstorage-file', async () => {
const cp = await spawnPromisified(process.execPath, [
'-pe', 'Object.keys(globalThis).includes("localStorage")',
]);
assert.strictEqual(cp.code, 0);
assert.match(cp.stdout, /false/);
});

test('localStorage is not persisted if it is unused', async () => {
Expand Down
Loading