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

feature(indexDB): Support indexDB for Safari browser #101

Closed
wants to merge 2 commits into from

Conversation

danielsogl
Copy link
Contributor

localForage 1.5.0 supports now indexDB for Safari browser: "We now use IndexedDB as the storage engine for Safari v10.1 (and above)."

localforage-cordovasqlitedriver updated also to localForage 1.5.0: thgreasi/localForage-cordovaSQLiteDriver#19

Daniel Sogl and others added 2 commits April 23, 2017 23:58
localForage 1.5.0 supports now indexDB for Safari browser: "We now use IndexedDB as the storage engine for Safari v10.1 (and above)."

localforage-cordovasqlitedriver updated also to localForage 1.5.0: thgreasi/localForage-cordovaSQLiteDriver#19
@jgw96
Copy link
Contributor

jgw96 commented May 12, 2017

Hey @danielsogl, while I love this PR (kill websql with 🔥 lol), this breaking change scares me a little. I am going to have to look into that more and see how it would potentially impact the community.

@danielsogl
Copy link
Contributor Author

@jgw96 would be nice!

@jesusbotella
Copy link

jesusbotella commented Aug 4, 2017

@jgw96 It may be a major version change, as localForage has done a breaking change :)

Did you gather feedback about the impact of the change? There is an issue that affects PWA in Safari private browsing mode (and maybe other browsers) that I am trying to address with the issue I opened in their repository and would be great to get it solved in ionic storage too when ready :)

@thgreasi
Copy link
Contributor

thgreasi commented Aug 5, 2017

@danielsogl I think that after updating to LF v1.5.0, you will no longer need the @types/localforage package (I think that it will also cause conflicts to your build).
As of the breaking change, this will not affect users of localForage-cordovaSQLiteDriver (android or iOS packed apps) but only pre-existing safari users of apps. I was caused by updating the browser sniffing regex that we used to detect & avoid the buggy implementation of IndexedDB in previous versions of safari.

If there was a confident way to detect whether the app runs for the first time or not + check whether it's on safari, then we could easily decide about using indexeddb or websql driver respectively.

Related discussion: localForage/localForage#676
PS: This looks similar to #78.

@thgreasi
Copy link
Contributor

@danielsogl, @jgw96, @jesusbotella
How would you feel for a compatibility plugin with the following api:

var driverPreferenceOrder = [
  localforage.INDEXEDDB,
  localforage.WEBSQL,
  localforage.LOCALSTORAGE
];

localforageCompatibility1_4.config().then(function() {
  localforage.config({
    driver: driverPreferenceOrder
  });
  // or
  // return localforage.setDriver(driverPreferenceOrder);
}).then(function() {
  // your LF code here
});

?
I think that such an API can be adopted from this repo.

@jesusbotella
Copy link

@thgreasi I think it would be really good and pretty easy to implement. So you use that localforageCompatibility1_4 plugin to maintain compatibility with the older version, right?

I suppose that it could be configurable inside ionic storage in order not to load the polyfill always, but only for the cases that the developer wants to make a transition from the older version to the new one.

@thgreasi
Copy link
Contributor

Yes that's right. The code is already there to check out but it depends on the not-yet-released LF v1.5.1, so I haven't published it. I'm currently waiting for a review on the respective LF PR and trying to gather as much feedback on the API if the compatibility plugin.

@thgreasi
Copy link
Contributor

@danielsogl localForage-compatibility-1-4 was just released to resolve the safari migration issue. Please check it out, I hope that you will like it.

@mhartington mhartington closed this Jun 8, 2018
@danielsogl danielsogl deleted the patch-1 branch June 8, 2018 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants