[shared_preferences] Adds support for Windows #2631

franciscojma86 · 2020-04-03T22:58:38Z

Description

Dart implementation of the shared_preferences plugin using ffi for Windows.

Related Issues

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
No, this is not a breaking change.

packages/shared_preferences/shared_preferences_windows/lib/shared_preferences_windows.dart

stuartmorgan

I just did a quick skim of the Dart code; not a full review.

Capturing from offline discussion:

We shouldn't check in the Windows example folder, since it isn't stable. (Instead, CI should flutter create it on demand, so it's always using the current template iteration.)
flutter create . makes a much of changes to older ios/ and android/ directories; all those changes should be removed from the PR.

packages/shared_preferences/shared_preferences_windows/lib/shared_preferences_windows.dart

packages/shared_preferences/shared_preferences_windows/lib/src/win32.dart

stuartmorgan

This is looking really good. There are a couple of questions we'll need to get input on, but I think it's really close.

packages/shared_preferences/shared_preferences/lib/shared_preferences.dart

stuartmorgan · 2020-04-09T21:31:23Z

packages/shared_preferences/shared_preferences/lib/shared_preferences.dart

-  static SharedPreferencesStorePlatform get _store =>
-      SharedPreferencesStorePlatform.instance;
+  static SharedPreferencesStorePlatform get _store {
+    if (LocalPlatform().isWindows) {


I forgot that the registration of the default implementation is in the platform interface rather than this layer; this override has the unfortunate effect that setting a different instance explicitly (e.g., the in-memory implementation, or an unofficial alternate version) won't work on Windows.

I wonder if we should temporarily add a static property to disable this, so that in those cases people can get the dynamic behavior with, e.g., SharedPreferencesStorePlatform.disablePlatformOverride = true; before calling getInstance(). Removing it later when we don't need it would technically be a breaking change, but it would be a very easy one for people to deal with (just remove the line of code). @amirh, thoughts?

stuartmorgan · 2020-04-09T21:32:11Z

packages/shared_preferences/shared_preferences_windows/LICENSE

@@ -0,0 +1,27 @@
+// Copyright 2019 The Chromium Authors. All rights reserved.


Does flutter/plugins not use an identical license everywhere like flutter/engine?

This license is the same as the one in shared_preferences, other than the year. Is that what you are referring to?

Specifically I was wondering if we are using a set year. flutter/engine does that, to avoid having a ton of license variations (which adds size to Flutter applications, since they include all the licenses).

packages/shared_preferences/shared_preferences_windows/android/src/main/AndroidManifest.xml

stuartmorgan · 2020-04-09T21:36:08Z

packages/shared_preferences/shared_preferences_windows/example/LICENSE

@@ -0,0 +1 @@
+TODO: Add your license here.


We should either have a license, or remove the file; it looks like both have been done. I'm not sure if one or the other is the preferred practice going forward so someone who knows should weigh in.

I just copied the license from the front-end package

packages/shared_preferences/shared_preferences_windows/lib/shared_preferences_windows.dart

franciscojma86 · 2020-04-10T22:28:48Z

All comments have been addressed. Now waiting on plugin_tools and Cirrus CI to be updated.
plugin_tools PR: https://github.com/flutter/plugin_tools/pull/95

Also waiting on @amirh comments for the class registration.

stuartmorgan

Just some license questions left from me.

stuartmorgan · 2020-04-15T16:39:05Z

packages/shared_preferences/shared_preferences_windows/LICENSE

@@ -0,0 +1,27 @@
+// Copyright 2019 The Chromium Authors. All rights reserved.


Specifically I was wondering if we are using a set year. flutter/engine does that, to avoid having a ton of license variations (which adds size to Flutter applications, since they include all the licenses).

packages/shared_preferences/shared_preferences_windows/example/LICENSE

...hared_preferences/shared_preferences_windows/example/test_driver/shared_preferences_e2e.dart

...ages/shared_preferences/shared_preferences_windows/test/shared_preferences_windows_test.dart

franciscojma86 · 2020-04-15T21:14:10Z

I changed the licenses to 2017 to match the rest of the repo. I have seen licenses with 2020 and 2019 in other packages, so I'm not really sure what's the policy. @amirh or @collinjackson might know?

GroovinChip · 2020-05-01T18:16:47Z

https://github.com/flutter/plugin_tools/pull/95 got merged, is anything else standing in the way of this PR being merged? I'm really excited for this to be official (I'm using it as a local path dependency and its great)

stuartmorgan · 2020-05-01T21:05:29Z

Filed flutter/flutter#56143 to track what's still blocking this PR.

packages/shared_preferences/shared_preferences_windows/lib/shared_preferences_windows.dart

packages/shared_preferences/shared_preferences_windows/pubspec.yaml

skyred · 2020-07-07T08:42:25Z

Can someone please provide instructions on how to use/test this PR with your app? I imagine add a specific reference/patch to this MR in pubspec.yaml

liudonghua123 · 2020-07-19T15:29:27Z

I hope this pr would be merged soon. 😄

skyred · 2020-08-15T12:18:26Z

I tried to use the PR, by specifying below in pubspec.yaml

  shared_preferences:
    git:
      url: git@github.com:franciscojma86/plugins.git
      path: ./packages/shared_preferences/shared_preferences/
      ref: shared

But, got this error:

Error on line 34, column 11: Invalid description in the "shared_preferences" pubspec on the "shared_preferences_windows" dependency: "../shared_preferences_windows" is a relative path, but this isn't a local pubspec.

   ╷

34 │     path: ../shared_preferences_windows

   │           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

   ╵
pub get failed (65;    ╵)
exit code 65

Haven't found a solution yet, can someone please help?

skyred · 2020-08-16T03:58:40Z

I had use this to make it working:

Step 1: git clone --branch shared git@github.com:franciscojma86/plugins.git --depth 1

Step 2: Update pubspec.yaml

dependencies:
  shared_preferences: '>=0.5.8 <2.0.0'
dependency_overrides:
  shared_preferences:
    path: ../plugins/packages/shared_preferences/shared_preferences/

Anyway, the functionality works in this PR.

gerryhigh · 2020-09-13T16:33:56Z

Getting weird issue with isLinux and isWindows not defined--see below

I had use this to make it working:

Step 1: git clone --branch shared git@github.com:franciscojma86/plugins.git --depth 1

Step 2: Update pubspec.yaml
dependencies:
  shared_preferences: '>=0.5.8 <2.0.0'
dependency_overrides:
  shared_preferences:
    path: ../plugins/packages/shared_preferences/shared_preferences/
Anyway, the functionality works in this PR.

I tried this but get an issue when trying to run it in my app:

/plugins/packages/shared_preferences/shared_preferences/lib/shared_preferences.dart(38,22): error G3824A48C: Getter not found: 'isLinux'.
/plugins/packages/shared_preferences/shared_preferences/lib/shared_preferences.dart(40,29): error G3824A48C: Getter not found: 'isWindows'

Flutter Doctor:
[✓] Flutter (Channel master, 1.22.0-10.0.pre.153, on Microsoft Windows [Version 10.0.19041.508], locale en-US)
• Flutter version 1.22.0-10.0.pre.153 at c:...
• Framework revision 2e64365 (2 days ago), 2020-09-11 23:07:03 -0400
• Engine revision 16b900b63e
• Dart version 2.10.0 (build 2.10.0-117.0.dev)
[✓] Visual Studio - develop for Windows (Visual Studio Community 2019 16.7.2)
• Visual Studio at C:\Program Files (x86)\Microsoft Visual Studio\2019\Community
• Visual Studio Community 2019 version 16.7.30413.136
• Windows 10 SDK version 10.0.18362.0
[✓] Connected device (1 available)
• Windows (desktop) • windows • windows-x64 • Microsoft Windows [Version 10.0.19041.508]

stuartmorgan · 2020-09-20T13:53:32Z

Closing as obsolete. Most of the implementation landed in #2988, it will be published when #3058 lands, and then #3059 will add the endorsement.

googlebot added the cla: yes label Apr 3, 2020

franciscojma86 changed the title ~~dShared~~ [shared_preferences] Adds support for Windows Apr 4, 2020

franciscojma86 mentioned this pull request Apr 4, 2020

[scripts] Exclude shared_preferences_windows package #2630

Merged

11 tasks

csells reviewed Apr 6, 2020

View reviewed changes

packages/shared_preferences/shared_preferences_windows/lib/shared_preferences_windows.dart Outdated Show resolved Hide resolved

stuartmorgan reviewed Apr 6, 2020

View reviewed changes

Remove files

d4a2904

franciscojma86 force-pushed the shared branch from dcd3a21 to d4a2904 Compare April 9, 2020 01:02

franciscojma86 added 5 commits April 8, 2020 18:40

Use path

230600c

Update comments

2f8e905

Format

1a99a1a

Remove create

1eb343d

fix

8f2881c

franciscojma86 marked this pull request as ready for review April 9, 2020 17:56

franciscojma86 requested review from amirh, stuartmorgan and csells April 9, 2020 17:56

stuartmorgan suggested changes Apr 9, 2020

View reviewed changes

Address messages

946cc68

stuartmorgan approved these changes Apr 15, 2020

View reviewed changes

License

4ae2926

This was referenced Apr 20, 2020

[shared_preferences] Add Windows support flutter/flutter#41719

Closed

[path_provider] path provider linux implementation in dart #2589

Merged

Overman775 mentioned this pull request May 16, 2020

Not working on windows aissat/easy_localization#189

Closed

stuartmorgan mentioned this pull request May 26, 2020

[path_provider] linux endorsement #2789

Merged

13 tasks

stuartmorgan mentioned this pull request Jun 3, 2020

[shared_preferences] Add Linux support flutter/flutter#41720

Closed

robert-ancell reviewed Jun 15, 2020

View reviewed changes

packages/shared_preferences/shared_preferences_windows/lib/shared_preferences_windows.dart Outdated Show resolved Hide resolved

robert-ancell reviewed Jun 15, 2020

View reviewed changes

packages/shared_preferences/shared_preferences_windows/pubspec.yaml Outdated Show resolved Hide resolved

robert-ancell reviewed Jun 16, 2020

View reviewed changes

packages/shared_preferences/shared_preferences_windows/pubspec.yaml Show resolved Hide resolved

robert-ancell mentioned this pull request Jun 18, 2020

[shared_preferences_linux] Add support for Linux #2836

Merged

13 tasks

joscdk mentioned this pull request Jul 1, 2020

Adding Windows support nstack-io/flutter-sdk#17

Closed

stuartmorgan added 2 commits September 1, 2020 08:36

Merge branch 'master' into shared

79a4f29

Minor fixes/updates

1da5569

stuartmorgan mentioned this pull request Sep 1, 2020

[shared_preferences] Add shared_preferences_windows #2988

Merged

13 tasks

stuartmorgan closed this Sep 20, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[shared_preferences] Adds support for Windows #2631

[shared_preferences] Adds support for Windows #2631

franciscojma86 commented Apr 3, 2020 •

edited

stuartmorgan left a comment

stuartmorgan left a comment

stuartmorgan Apr 9, 2020

stuartmorgan Apr 9, 2020

franciscojma86 Apr 9, 2020

stuartmorgan Apr 15, 2020

stuartmorgan Apr 9, 2020

franciscojma86 Apr 10, 2020

franciscojma86 commented Apr 10, 2020

stuartmorgan left a comment

stuartmorgan Apr 15, 2020

franciscojma86 commented Apr 15, 2020

GroovinChip commented May 1, 2020 •

edited

stuartmorgan commented May 1, 2020

skyred commented Jul 7, 2020

liudonghua123 commented Jul 19, 2020

skyred commented Aug 15, 2020

skyred commented Aug 16, 2020

gerryhigh commented Sep 13, 2020 •

edited

stuartmorgan commented Sep 20, 2020

		@@ -0,0 +1,27 @@
		// Copyright 2019 The Chromium Authors. All rights reserved.

[shared_preferences] Adds support for Windows #2631

[shared_preferences] Adds support for Windows #2631

Conversation

franciscojma86 commented Apr 3, 2020 • edited

Description

Related Issues

Checklist

Breaking Change

stuartmorgan left a comment

Choose a reason for hiding this comment

stuartmorgan left a comment

Choose a reason for hiding this comment

stuartmorgan Apr 9, 2020

Choose a reason for hiding this comment

stuartmorgan Apr 9, 2020

Choose a reason for hiding this comment

franciscojma86 Apr 9, 2020

Choose a reason for hiding this comment

stuartmorgan Apr 15, 2020

Choose a reason for hiding this comment

stuartmorgan Apr 9, 2020

Choose a reason for hiding this comment

franciscojma86 Apr 10, 2020

Choose a reason for hiding this comment

franciscojma86 commented Apr 10, 2020

stuartmorgan left a comment

Choose a reason for hiding this comment

stuartmorgan Apr 15, 2020

Choose a reason for hiding this comment

franciscojma86 commented Apr 15, 2020

GroovinChip commented May 1, 2020 • edited

stuartmorgan commented May 1, 2020

skyred commented Jul 7, 2020

liudonghua123 commented Jul 19, 2020

skyred commented Aug 15, 2020

skyred commented Aug 16, 2020

gerryhigh commented Sep 13, 2020 • edited

stuartmorgan commented Sep 20, 2020

franciscojma86 commented Apr 3, 2020 •

edited

GroovinChip commented May 1, 2020 •

edited

gerryhigh commented Sep 13, 2020 •

edited