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

fix(Cache): typescript Map generic type fixed #308

Open
wants to merge 50 commits into
base: master
Choose a base branch
from

Conversation

dimaslanjaka
Copy link
Contributor

@dimaslanjaka dimaslanjaka commented May 14, 2023

feat: add CacheMapper

implement built-in javascript Map with full function usages.

  • Create new class CacheMapper implementing Map<KEY, VALUE> which is genericable
  • Expose all the functionality of the Map class
  • Add private inner Map<string, V> to prevent variable class imported using private name, which V is Generic type
  • class Cache<V> implementing CacheMapper<string, V>

fix: generic type Cache class

  • hotfixed Exported variable 'cache' has or is using private name 'cache'

because type any is not allowed by ESLint so I created a mapper class with full typeStrong which can be generalized

fixed generic type screenshot
image

chore(deps-dev): add @types/chai and @types/mocha

to support mocha, chai for used in typescript, we need @types/chai and @types/mocha

feat: add strict type test for Cache

define cache class with various type (still on-going creating an tests)

make `value` and `returns` as generic type
- disable @typescript-eslint/no-explicit-any
to be able treat `value` as any
- fixed failed build caused by `Not all constituents of type 'T | function' are callable`
cache now full support like built-in `Map`

forked from: https://stackoverflow.com/a/68805082/6404439
detach `dump` from CacheMapper
chore(deps-dev): add `@types/chai` `@types/mocha`
refactor(eslintrc): disable unsupported es-syntax for typescript
feat: import `chai.expect`

fixed for objects not being sorted by NodeJS built-in `Map`
fix: CacheMapper using private name
@dimaslanjaka dimaslanjaka marked this pull request as draft May 14, 2023 03:00
@coveralls
Copy link

coveralls commented May 14, 2023

Coverage Status

Coverage: 95.047% (-0.4%) from 95.402% when pulling ddb89ec on dimaslanjaka:fix-cache into 3b0bc15 on hexojs:master.

@dimaslanjaka
Copy link
Contributor Author

dimaslanjaka commented May 14, 2023

@SukkaW can i add typescript test for generic type of Cache ?

i want add more tests within various type which Cache fully supported generic type.

generic cache used for improving hexo locals which successful cast the get return generic
image
image

and I need solve my test issue #309, how to fix this?

lib/CacheMapper.ts Outdated Show resolved Hide resolved
const Cache = require('../dist/cache');
const cache = new Cache();

it('get & set', () => {
cache.set('foo', 123);
cache.get('foo').should.eql(123);
should.equal(cache.get('foo'), 123);
Copy link
Member

@SukkaW SukkaW May 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@types/chat plus import {should} 'chat'; should(); should be enough to correct the type of [any object].prorotype.should, just like this: https://github.com/SukkaW/rollup-plugin-swc/blob/master/test/index.ts

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@types/chat plus import {should} 'chat'; should(); should be enough to correct the type of [any object].prorotype.should, just like this: https://github.com/SukkaW/rollup-plugin-swc/blob/master/test/index.ts

at my device using prototype.should getting error (throw undefined and make all tests failed), instead giving result test failure

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i dont know, why. But using normal chai API can give result failure correctly instead throw all tests runner, that was made me confused, because throw from node_modules paths instead from the tests files

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

emm, i think this error caused by running on monorepo/workspace project. cuz all dependents grouped at single node_modules. but, this method easily to sync between packages. maybe, caused by version conflict (auto fixed by yarn), i dont know. Remembering all hexo packages has different dev dependencies versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error says:

cannot read property 'should' of undefined

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sometimes got error like:

cannot read property 'eql' ... so on

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so to prevent useless error, prefer using original chai expect, should API for safety :)

dimaslanjaka added a commit to dimaslanjaka/hexo that referenced this pull request May 15, 2023
@dimaslanjaka dimaslanjaka marked this pull request as ready for review May 15, 2023 17:45
@dimaslanjaka dimaslanjaka requested a review from SukkaW May 15, 2023 17:48
dimaslanjaka added a commit to dimaslanjaka/hexo that referenced this pull request May 17, 2023
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.

None yet

3 participants