-
Notifications
You must be signed in to change notification settings - Fork 2
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
chore(deps): update all deps and fix database schema #37
Conversation
47c803d
to
af31115
Compare
@@ -1,7 +1,7 @@ | |||
{ | |||
"name": "unity-cache", | |||
"version": "2.1.0", | |||
"description": "Cache abstraction around localforage.", | |||
"version": "2.2.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You updated node to major version. I think it should been updated to major.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Node is used only in testing environment. I'm not sure about the major version.
"version": "2.1.0", | ||
"description": "Cache abstraction around localforage.", | ||
"version": "2.2.0", | ||
"description": "Cache abstraction around Dexie.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
config: {}, | ||
db: null | ||
db: null, | ||
config: {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Useless change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's like a tree 🌲
stores = stores.reduce((result, storeName) => { | ||
if (!RE_BIN.test(storeName)) { | ||
throw new UnityCacheError(`Store names can only be alphanumeric, '${storeName}' given`); | ||
} | ||
|
||
result[storeName] = '&'; | ||
result[storeName] = '&key, value, expire'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To remove EXPIRE_BIN
const { db } = cacheInstance; | ||
const expire = Date.now() + Number(ttl); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Number('fdfff') // NaN
I think it would be better to check its type before
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better to rewrite it to typescript. The check doesn't help at all.
|
||
const indexedDB = require('fake-indexeddb'); | ||
const FDBKeyRange = require('fake-indexeddb/lib/FDBKeyRange'); | ||
|
||
window.Promise = global.Promise; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cause Dexie uses Promise from window.
No description provided.