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

Types #22

Merged
merged 8 commits into from
Nov 11, 2019
Merged

Types #22

merged 8 commits into from
Nov 11, 2019

Conversation

FFKL
Copy link
Contributor

@FFKL FFKL commented Nov 3, 2019

Hello! I've added types, but it broke backward compatibility for imports. Review please this version. I thinking about saving current imports with entry point, but now I have problems with it 😄

@bahung1221
Copy link
Owner

Cool, thanks bro 😄 ,

I think the CacheAll.Cacher should be an interface instead class, because we just call cache.get() without new Cache(...).

I also think we should have three d.ts files for three entry points (memory.js, file.js, redis.js) instead one entry point, is it possible to have an config.ts (or something) that export CacheAll namespace, and then it will be import into three d.ts entry point?

Currently, i separate entry points to create independent singleton cache instance, i will research to find more ways to archive it.

@FFKL
Copy link
Contributor Author

FFKL commented Nov 5, 2019

Ok. I'll try create separate files 😄 Doesn't import from index.js file create singleton?

@bahung1221
Copy link
Owner

bahung1221 commented Nov 5, 2019

Ok. I'll try create separate files 😄 Doesn't import from index.js file create singleton?

No, it does.
But we must create three instance regardless the users just want to use one. Currently, in each entry point i exported an returned instance from an function, and each entry point will be call only when user require it, if they require just one entry (redis for example), only one cache instance will be create.

module.exports = require('./src/cache')('file')

If we use index.js file, we must do something like below:

// Maybe user just want to use one, but three instance was created
module.exports.file = require('./src/cache')('file')
module.exports.memory = require('./src/cache')('memory')
module.exports.redis = require('./src/cache')('redis')

Ofcourse, the effect is too small 😄

@FFKL
Copy link
Contributor Author

FFKL commented Nov 9, 2019

Hello! I've reworked types. Now it works with 4 d.ts files, but they are identical 😄 Check please. Have you some suggestions?

@bahung1221
Copy link
Owner

Hi bro, i think it fine, i just split config.d.ts and i have no idea about identical functions.
Please help me review my commit, i tested and it worked, but is it has any problem? 😄

@FFKL
Copy link
Contributor Author

FFKL commented Nov 10, 2019

@bahung1221 I think, i found right way for types 😄 Check please. Same functions for all modules

@bahung1221
Copy link
Owner

Cool! i have some problems when try to test it using relative path, lol. so i will release an beta version of it and then try to make it run in example project, it will be 2.1.0-beta.0

Thank bro 😄

@bahung1221 bahung1221 merged commit 243249d into bahung1221:master Nov 11, 2019
@FFKL FFKL deleted the types branch November 11, 2019 12:52
@bahung1221
Copy link
Owner

#2

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.

2 participants