Web implementation of shared_preferences#2332
Conversation
| description: Web platform implementation of shared_preferences | ||
| author: Flutter Team <[email protected]> | ||
| homepage: https://github.com/flutter/plugins/tree/master/packages/shared_preferences/shared_preferences_web | ||
| version: 0.0.1 |
There was a problem hiding this comment.
Nit: prefer the initial release to be 0.1.0 to get a better score on pub
ditman
left a comment
There was a problem hiding this comment.
This looks great! I just suggested a few changes, but nothing blocking IMO.
packages/shared_preferences/shared_preferences_web/CHANGELOG.md
Outdated
Show resolved
Hide resolved
| @override | ||
| Future<bool> setValue(String valueType, String key, Object value) async { | ||
| _checkPrefix(key); | ||
| html.window.localStorage[key] = _encodeValue(value); |
There was a problem hiding this comment.
Does it make sense to prepend something to the user-supplied key, to minimize collisions with other things that might be using the shared local storage? shared_preferences or similar?
We can make this transparent on all add/remove operations so users don't even see that we're doing this (unless they go to the dev tools and peek :P)
There was a problem hiding this comment.
The SharedPreferences class from package:shared_preferences already prepends "flutter.". So here I'm just verifying that it's prefixed in _checkPrefix. On top of that, for security reasons, localStorage is scoped to the domain, so you can't step on another site's data.
We could allow customizing the prefix, but currently the value is hard-coded across Dart, Java, and Objective-C code, so it would be a much bigger change.
There was a problem hiding this comment.
Ah ok, I thought this relied on the user prepending "flutter." to the key themselves. If it's the shared_prefs plugin, I guess it's OK.
I know you can't step on other's site data; the scenario I was envisioning was multiple plugins writing to localStorage on the same site, and overwriting each other's data.
packages/shared_preferences/shared_preferences_web/lib/shared_preferences_web.dart
Show resolved
Hide resolved
|
|
||
| environment: | ||
| sdk: ">=2.0.0-dev.28.0 <3.0.0" | ||
| flutter: ">=1.5.0 <2.0.0" |
There was a problem hiding this comment.
We haven't been too strict with this so far, but I think that you need a newer version of flutter to support the plugin.platforms syntax of the pubspec.yaml.
I'm not sure what's a good value for the minimal version of flutter yet, though :)
There was a problem hiding this comment.
It's a copy'n'paste from url_launcher. LMK what I should use here instead.
There was a problem hiding this comment.
I'm not entirely sure, I have a feeling we'll end up coming and updating all of these to the proper version later on, don't worry too much! :P
packages/shared_preferences/shared_preferences_web/test/shared_preferences_web_test.dart
Show resolved
Hide resolved
| } | ||
|
|
||
| // Check that generics are preserved. | ||
| expect((await store.getAll())['flutter.StringList'], isA<List<String>>()); |
There was a problem hiding this comment.
@ditman this test here shows how the generics are preserved. If you change this to isA<List<int>>(), the test will fail.
There was a problem hiding this comment.
This is a functionality from the language that I wasn't expecting at all!
| dependencies: | ||
| ... | ||
| shared_preferences: ^0.5.4+8 | ||
| shared_preferences_web: ^0.0.1 |
Implement `shared_preferences` for the Web.
Description
Implement Web support for
package:shared_preferenceson top ofwindow.localStorage.Related Issues
Fixes flutter/flutter#35116