Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Implement localStorage #1657

Closed
dsseng opened this issue Feb 2, 2019 · 30 comments · Fixed by #7819
Closed

Implement localStorage #1657

dsseng opened this issue Feb 2, 2019 · 30 comments · Fixed by #7819
Labels
cli related to cli/ dir suggestion suggestions for new features (yet to be agreed)

Comments

@dsseng
Copy link
Contributor

dsseng commented Feb 2, 2019

Implement localStorage and sessionStorage following the MDN spec, sessionStorage

@kitsonk
Copy link
Contributor

kitsonk commented Feb 2, 2019

In #1181 we discussed that these could be external modules added to the Deno registry and there wasn't a need for them to be part of the core of Deno.

@ry
Copy link
Member

ry commented Feb 7, 2019

I changed my mind. I think local storage would be really useful for serverless situations - imagine you have a server and you want to store a simple chat log or counter - with localStorage we can circumvent using a database nor expose the disk. Let's do it!

This was referenced Feb 7, 2019
@akmittal
Copy link

Wont it be better to support sqlite by default, Something like android does. localStorage would be too limited.

@kitsonk
Copy link
Contributor

kitsonk commented Feb 11, 2019

localStorage is a published API that people expect to be there. IndexDB is another. Supporting sqlite is actually tangental to supporting browser standard APIs.

@ry
Copy link
Member

ry commented Feb 20, 2019

https://developer.mozilla.org/en-US/docs/Web/API/Cache
Would be cool to have this as a lower-level storage that localStorage is implemented on top of.

Also- should be noted that the reason we want this in core is that Cache/localStorage should be accessible even when --allow-read and --allow-write are off.

@zekth
Copy link
Contributor

zekth commented Mar 4, 2019

What's the policy for the cache? Is it persistent if the runtime turns off or not?

@ry
Copy link
Member

ry commented Mar 4, 2019

@zekth it's persistent

@zekth
Copy link
Contributor

zekth commented Mar 4, 2019

So if we use something like indexDB it will be shared by all scripts using the Deno runtime? If so, have to ensure there is no overlapping. Also do we add any maximum cache size or a permission for the max size?

@kitsonk
Copy link
Contributor

kitsonk commented Mar 4, 2019

I believe they are part of the spec or defacto standards in browser implementations.

@zekth
Copy link
Contributor

zekth commented Mar 5, 2019

@axetroy
Copy link
Contributor

axetroy commented Dec 16, 2019

In Chrome, LocalStorage uses leveldb

Maybe implement leveldb before implementing LocalStorage

Although the specification does not specify what db to used

@axetroy
Copy link
Contributor

axetroy commented Dec 18, 2019

I am trying to do this with https://docs.rs/rusty-leveldb/0.3.0/rusty_leveldb/index.html to implement localStorage

.
└── localstorage
    └── deno.land
        ├── 000003.log
        ├── CURRENT
        ├── LOCK
        ├── LOG
        └── MANIFEST-000001

2 directories, 5 files

Does localStorage need to be persistent?

  • no

same with sessionStorage and easy implement in Typescript side

  • yes

How to isolate files?

localStorage isolate files by domain name in browser.

Different domain names have different leveldb.

But in Deno, it is common sense to isolate files with pid

Because it is unique, but each time the pid is different when start a Deno process, it cannot be persisted

So is there a better isolation solution?

eg. run command line with this deno run --domain=deno.land xxx.ts

If files are not isolated

all Deno processes use the same file

Writing files so frequently may cause errors and is not safe

This is the problem I have to resolve.

@davidbarratt
Copy link

davidbarratt commented Dec 18, 2019

in the browser, localStorage is persisted across processes and is subject to the same-origin policy... though, maybe we add an --origin (which seems more appropriate than --domain) paremeter to the CLI to scope the storage, and if that is not provided, it acts like sessionStorage (i.e. it gets deleted when the process ends?)

@davidbarratt
Copy link

This gave me an idea.... maybe it should be scoped to the file that is being executed? Though if you change the location of the file that would break the localStorage but maybe that is ok?

@dsseng
Copy link
Contributor Author

dsseng commented Dec 18, 2019

The app could probably request access for origins, then the user will allow/disallow that access.

@davidbarratt
Copy link

The app could probably request access for origins, then the user will allow/disallow that access.

Right, that would basically be exposing this:
https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/origin
though, not sure why you would need permission to access that?

@kitsonk
Copy link
Contributor

kitsonk commented Feb 1, 2020

From @rektide

Pardon this is rude and or unhelpful, but I might recommend Sled as the hot new LevelDB-ish replacement that's getting a ton of active development, looks super fast, is cross platform, in general, looks super hip.
https://github.com/spacejam/sled

@teleclimber
Copy link

Copy-pasting my comment from #4671 since this thread is more appropriate:

Please make it possible to disable access to localStorage and indexDB and anything else that has side-effects that gets added to Deno in the future. The reason is that things get a lot more complicated for those of us using Deno as the sandbox component of an application platform when apps can have side-effects that we can't control.

Example: I might use Deno to run code on demand on any of a large number of VMs. If an app stashed some data in locaStorage during one run, it's going to expect that to be there on a subsequent run. But that might happen on a totally different VM, so the data won't be there.

At least make it possible to export all localStorage/IndexDB data for an origin from the command line, and provide an import function as well.

One more concern is that it creates the need for an additional layer of control around the amount of data that an origin/whatever can store in their local storage. So long as deno only allowed access to the underlying OS file system, a host could could use the OS's facilities to track and control the user's storage usage. Once you start throwing localStorage data from every app into some internal db, the burden falls on deno to track, report, and limit storage usage.

Thanks

@kitsonk
Copy link
Contributor

kitsonk commented Apr 11, 2020

@teleclimber anything that writes/persists data, outside of normal imports, would/should require some sort of allow flag. Security definitely needs to be a consideration for this.

@nayeemrmn
Copy link
Collaborator

I'm pretty sure the "persistent-storage" permission exists on the web, and would dictate whether or not the localStorage entries are actually persisted. Unless I've misunderstood that, no problems there.

@davidbarratt
Copy link

davidbarratt commented Apr 16, 2020

The browser doesn't require permission to use localStorage (up to a quota amount which I believe is typically 50mb).

I'm not sure why Deno would need permission to store data in a /tmp style directory (up to a similar quota), since, like the browser, that storage is effectively "temporary".

See https://developer.mozilla.org/en-US/docs/Web/API/Storage_API for more details.

With the persistent-storage permission, data (in any storage system) can be persisted beyond it's temporary state.

@Caesar2011
Copy link
Contributor

Local Storage in a browser can be removed by the user and therefore the web application cannot rely on it. The same states for Deno. The program may be be copied to another host or the local DB cache is cleared. A web app should be a able to handle such cases.

@MarkTiedemann
Copy link
Contributor

MarkTiedemann commented Apr 30, 2020

Something that I have not seen mentioned in this debate yet is that localStorage exposes only sync APIs. So I'd question whether you'd actually use it in most server settings.

There's an async version called kvStorage (see: https://github.com/WICG/kv-storage), but work on it is currently suspended "as no browser teams (including the Chromium project, which originated the proposal) are currently indicating interest in implementing it".

Then there's the argument that "with localStorage we can circumvent using a database nor expose the disk". Yes, you'd save --allow-read/write for disk access or --allow-net for DB access, but at the same time it seems like that at least one other flag (e.g. --location) needs to be added for this use case. So I'm not sure that's a good trade-off - I'd be in favor of having as few flags as possible.

Also, I'm not sure I'd like localStorage to be intransparent about where it's actually persisting stuff to disk. Sure, in the browser that's totally fine. But for my scripts or servers, I'd like to know. Does it write into $cwd, /tmp, $DENO_DIR, $RANDOM?

I actually think a third party, or even standard module, with a transparent API might be better, for example:

import { initLocalStorage } from "https://deno.land/std@v0.41.0/web-compat/localstorage.ts";

async function main() {
    let localStorage = await initLocalStorage("my_local_storage.db");
    localStorage.setItem("a", "b");
    // ...
}

main();

On second thought, I see no reason for providing sessionStorage other than web compat. There's no sessions; no refreshing the tab. The abstraction seems kinda leaky in this context. Also, if there are no sessions and you don't need actual persistence, why not just use a good old Map?

@aduh95
Copy link
Contributor

aduh95 commented Apr 30, 2020

localStorage exposes only sync APIs

Also it is not available on web workers.

IndexedDB would be a stronger contender for the use case discussed here IMO, but boy is that not a small API! (see #1699 (comment))

@crowlKats
Copy link
Member

So i started working an a localStorage implementation, but there is an issue: which DB to use.
leveldb and sled wont work because they allow only one connection at a time, making it not possible to use the same origin twice.
In the discord we considered SQLite, as it could be used for a future implementation of iDB, which would result in just a single dependency for these kind of storages.
What db should we go with?

@ghost
Copy link

ghost commented Sep 17, 2020

lmdb is another good alternative

@kryptish
Copy link

Just a JSON file =)?

@exside
Copy link
Contributor

exside commented Sep 17, 2020

@kryptish I actually implemented something in this direction a while ago, it tries to mirror the Browser localStorage interface as closely as possible and just saves (by default, can be configured via constructor) a .json file with the data in the working directory (it creates a .tmp folder), The interface mirroring works pretty well, except for the console.log() which is different due to the internal use of a Map, but I thought that didn't really matter, also it doesn't support quotas and does not emit events and it needs --allow-read and --allow-write flags:

import {
	copySync,
	ensureFileSync,
	existsSync
} from 'https://deno.land/std/fs/mod.ts';

// maybe a Deno flag like --persistent-storage would be useful in the future
// https://storage.spec.whatwg.org/#persistence
// interface: https://html.spec.whatwg.org/multipage/webstorage.html#the-storage-interface
export default class Storage {
	// Can't use privat fields and Proxy(), things will crash hard, see
	// https://disq.us/url?url=https%3A%2F%2Fgit.luolix.top%2Ftc39%2Fproposal-class-fields%2Fissues%2F106%3ACgK5-2pGsZhNCXXqGKGy2OO0PwI&cuid=611304
	//#entries;

	constructor(path) {
		// should this even be configurable?
		Reflect.defineProperty(this, 'path', {
			configurable: false,
			enumerable: false,
			writable: false,
			value: path || `${Deno.cwd()}/.tmp/localStorage.json`,
		});

		Reflect.defineProperty(this, 'entries', {
			configurable: false,
			enumerable: true,
			writable: true,
			value: null,
		});

		Reflect.defineProperty(this, 'read', {
			configurable: false,
			enumerable: false,
			writable: false,
			value: function() {
				if ( existsSync(this.path) ) {
					try {
						return Object.entries(JSON.parse(Deno.readTextFileSync(this.path)));
					} catch(err) {
						// check for backup
						if ( existsSync(`${this.path}.backup`) ) {
							try {
								return Object.entries(JSON.parse(Deno.readTextFileSync(`${this.path}.backup`)));
							} catch(err) {
								return [];
							}
						}
					}
				} else {
					return [];
				}
			},
		});

		Reflect.defineProperty(this, 'write', {
			configurable: false,
			enumerable: false,
			writable: false,
			value: function() {
				ensureFileSync(this.path);
				// create backup in case something goes wrong while writing the file
				// Deno crashing mid-write or something similar can cause corrupted JSON!
				copySync(this.path, `${this.path}.backup`, {
					overwrite: true,
					preserveTimestamps: true,
				});
				// persist to disk...
				Deno.writeTextFileSync(this.path, JSON.stringify(Object.fromEntries(this.entries)));
			},
		});

		Reflect.defineProperty(this, 'delete', {
			configurable: false,
			enumerable: false,
			writable: false,
			value: function() {
				[this.path, `${this.path}.backup`].forEach((path) => {
					existsSync(path) && Deno.removeSync(path);
				});
			},
		});

		/**
		 * Returns the number of key/value pairs. In the Browser this is enumerable!
		 * Plus setting length on Browser localStorage with `localStorage['length'] = x`
		 * creates an actual entry with key 'length'...but then the console.log() representation
		 * changes and contains an entries property.
		 * 
		 * 
		 * @returns {Number}
		 */
		Reflect.defineProperty(this, 'length', {
			configurable: true,
			enumerable: true,
			writeable: true,
			get: function() {
				return this.entries.size;
			},
		});


		this.entries = new Map(this.read());

		return new Proxy(this, {
			get(target, key, receiver) {
				
				if ( !target[key] ) {
					return target.getItem(key);
				}
				return Reflect.get(target, key, receiver);
			},
			set(target, key, value) {
				// redirect setting any properties on the Storage object itself to the Map
				target.setItem(key, value);

				return true;
			},
			deleteProperty(target, key) {
				target.removeItem(key);

				return true;
			}
		});
	}

	/**
	 * Returns the name of the nth key, or null if n is greater than or equal to 
	 * the number of key/value pairs.
	 * 
	 * @param {Number} index
	 * 
	 * @returns {String}
	 */
	key(index) {
		return index >= this.entries.length ? null : [...this.entries.keys()][index];
	};

	/**
	 * Returns the current value associated with the given key,
	 * or null if the given key does not exist.
	 * 
	 * @param {String} key
	 * 
	 * @returns {String}
	 */
	getItem(key) {
		if ( this.entries.has(key) ) {
			return String(this.entries.get(key));
		}
		return null;
	};

	/**
	 * Sets the value of the pair identified by key to value, creating a new key/value pair
	 * if none existed for key previously.
	 * TODO: Throws a "QuotaExceededError" DOMException exception if the new value couldn't be set.
	 * (Setting could fail if, e.g., the user has disabled storage for the site,
	 * or if the quota has been exceeded.)
	 * TODO: Dispatches a storage event on Window objects holding an equivalent Storage object.
	 * 
	 * @note Browser's behaviour is a bit strange, with localStorage[<key>] = <value> it returns
	 *       the value, but when using localStorage.setItem(<key>) it always returns undefined.
	 * 
	 * @param {String} key 
	 * @param {any} value 
	 * 
	 * @returns undefined
	 */
	setItem(key, value) {
		this.entries.set(key, String(value));
		this.write();
	};

	/**
	 * Removes the key/value pair with the given key, if a key/value pair with the given key exists.
	 * TODO: Dispatches a storage event on Window objects holding an equivalent Storage object.
	 * 
	 * @note Browser's behaviour is a bit strange, with delete localStorage[<key>] it returns true
	 *       for known and unknown keys, but when using localStorage.removeItem(<key>) it always
	 *       returns undefined, regardless if the key was known or not.
	 * 
	 * @param {String} key 
	 * 
	 * @returns undefined
	 */
	removeItem(key) {
		this.entries.delete(key);
		this.write()
	};

	/**
	 * Removes all key/value pairs, if there are any.
	 * TODO: Dispatches a storage event on Window objects holding an equivalent Storage object.
	 * 
	 * @returns undefined
	 */
	clear() {
		this.entries.clear();
		this.delete();
	};
}

some primitive tests:

import { default as Storage } from './Storage.js';

const localStorage = new Storage();
console.log(localStorage);

localStorage.setItem('int', 0);
localStorage.setItem('float', 1/7);
localStorage.setItem('array', [1, 'two', 3, 'four']);
localStorage.setItem('json', JSON.stringify({'some': 'object', 'with': 1}));
localStorage['some'] = 'value';

// some strange edge cases
localStorage['length'] = 0;
console.log('localStorage["set"] = "a value" =>', localStorage['set'] = 'a value');

console.log('localStorage.key(2) =>', localStorage.key(2));
console.log('getItem("array") =>', localStorage.getItem('array'));
console.log('localStorage["array"] =>', localStorage['array']);

console.log(localStorage);

console.log('delete known key with delete =>', delete localStorage['json']);
console.log('delete unknown key with delete =>', delete localStorage['unknown']);
console.log('delete known key that exists on the Storage object as well with delete =>', delete localStorage['length']);
console.log('delete unknown key with removeItem() =>', localStorage.removeItem('unknown'));
localStorage.removeItem('set');

console.log(localStorage);

// localStorage.clear();

// console.log('clear() =>', localStorage);

@kitsonk kitsonk added cli related to cli/ dir feat new feature (which has been agreed to/accepted) labels Sep 25, 2020
@ghost
Copy link

ghost commented Oct 13, 2020

Do you suggest limitations like that of the browser? Ex: 2MB - 5MB memory cap?

@crowlKats
Copy link
Member

@00ff0000red yes, limit is going to be 5MB, just like in the spec.

@bartlomieju bartlomieju added suggestion suggestions for new features (yet to be agreed) and removed feat new feature (which has been agreed to/accepted) labels Nov 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

Successfully merging a pull request may close this issue.