feat(typescript): migrate to typescript #56

MrSwitch · 2023-02-02T07:59:03Z

Migrate to TS

MrSwitch · 2023-02-03T10:55:42Z

src/index.ts

+/* eslint-disable @typescript-eslint/ban-types */
+interface MemoizeOptions {
+	// useCached: Will return the last resolved cached value
+	useCached?: true | false | Function;


Question: How do i say that this Function would be called with a single parameter matching CacheItem?

would (param: CacheItem) => void work?

Thanks @Mejsun 👍

MrSwitch · 2023-02-03T12:06:52Z

src/index.ts

+	getKey?: Function;
+
+	// cache: Caching Map
+	cache?: Map<string | number | symbol, any>;


This Map key definition seems a bit verbose, and possibly incomplete, does anyone know a better way?

MrSwitch · 2023-02-03T12:09:16Z

src/index.ts

+
+	// getKey: Default key definition
+	// eslint-disable-next-line @typescript-eslint/ban-types
+	getKey?: Function;


Function should define any function which takes an an unknown number and type of parameters. And returns something that can be used as a Map Key. How would i define such a thing... and would it be just as flexible as Function here?

MrSwitch · 2023-02-06T11:39:44Z

src/index.ts

 	 * @returns {Promise} Value of the callback
 	 */
-	return async (...args) => {
+	return async (...args: any) => {


Question: How do i remove the any here?

I want it to infer the parameter Types which are acceptable by the other parameter function callback.

You'll need a generic for TS to infer this.

Define the memoise function like so:

export default function Memoize<ParameterTypes extends Array<unknown>>( callback: (...args: ParameterTypes) => unknown, opts: MemoizeOptions = {} ) {

Then you can use the ParameterTypes generic here:

Suggested change

return async (...args: any) => {

return async (...args: ParameterTypes) => {

Might also be worth replacing all the anys in this file with unknown, TS is stricter about what you can do with that, and if you don't really care about those types, it's just as good.

If you want to keep the separate MemoizeCallback type, define it like so and pass through the generic:

type MemoizeCallback<ParameterTypes extends Array<unknown>> = ( ...args: ParameterTypes ) => unknown; export default function Memoize<ParameterTypes extends Array<unknown>>( callback: MemoizeCallback<ParameterTypes>, opts: MemoizeOptions = {} ) {

But I'd argue it's not worth it.

Another thing I just thought about – it might be nice to be able to work with the return type of the callback, eg. like so:

export default function Memoize< ParameterTypes extends Array<unknown>, CallbackValue extends Promise<unknown> >( callback: (...args: ParameterTypes) => CallbackValue, opts: MemoizeOptions<ParameterTypes, CallbackValue> = {} ) {

Though this implies the function can only be used with async functions, not sure if that is intended.

In any case this will make sure that the memoised function keeps its return type.

diondiondion · 2023-02-07T16:27:11Z

src/index.ts

+interface MemoizeOptions {
+	// useCached: Will return the last resolved cached value
+	useCached?: true | false | MemoizeUseCacheHandler;
+
+	// staleInMs: How long before we should check for new updates
+	staleInMs?: number;
+
+	// getKey: Default key definition
+	getKey?: (...args: any) => MemoizeCacheKey;


You can also weave the ParameterTypes from my earlier suggestion through to the options and use it there:

Suggested change

interface MemoizeOptions {

// useCached: Will return the last resolved cached value

useCached?: true | false | MemoizeUseCacheHandler;

// staleInMs: How long before we should check for new updates

staleInMs?: number;

// getKey: Default key definition

getKey?: (...args: any) => MemoizeCacheKey;

interface MemoizeOptions<ParameterTypes extends Array<unknown>> {

// useCached: Will return the last resolved cached value

useCached?: true | false | MemoizeUseCacheHandler;

// staleInMs: How long before we should check for new updates

staleInMs?: number;

// getKey: Default key definition

getKey?: (...args: ParameterTypes) => MemoizeCacheKey;

diondiondion · 2023-02-07T16:31:35Z

src/index.ts

 		// cache Max Size
 		cacheMaxSize = 1000,
-	} = opts;
+	}: MemoizeOptions = opts;


TS should be able to infer this even if you remove it.

diondiondion · 2023-02-07T16:37:54Z

src/index.ts

-	return async (...args) => {
+	return async (...args: any) => {
 		// Serialize the keys...
 		const key = getKey(...args);


Interestingly, TS flags this with the error A spread argument must either have a tuple type or be passed to a rest parameter. And it seems to have a point: If the callback function has multiple parameters, does it make sense to pass those to e.g. JSON.stringify as individual params?

Maybe they should be passed like so?

Suggested change

const key = getKey(...args);

const key = getKey(args);

I tried your implementation... this certainly proves a sticking point. I would like to to have the same parameter structure as the callback function. But yes, Typescript is happier if i pass this through as an array of the first parameter

Yeah, maybe there's a way to shut up that TS error.

BREAKING CHANGE: this changes the getKey parameters 😭

# [2.0.0](v1.4.7...v2.0.0) (2023-03-17) ### Features * **typescript:** migrate to typescript ([#56](#56)) ([52fa531](52fa531)) ### BREAKING CHANGES * **typescript:** this changes the getKey parameters 😭 * chore(deps): update deps * feat(ts): add callback values, noissue * chore: rm editorconfig * chore(lint): fix lint * chore(ts): remove deprecation warnings BREAKING CHANGES: Change in the `options.getKey(...args)` => `options.getKeys(args)` args is contained in first parameter.

5app-Machine · 2023-03-17T12:49:26Z

🎉 This PR is included in version 2.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Andrew Dodson added 3 commits February 2, 2023 07:56

feat(typescript): migrate to typescript

298c963

chore: merge

aec0e3c

chore(lint): fix lint-diff

5742c57

MrSwitch self-assigned this Feb 2, 2023

MrSwitch requested a review from a team February 2, 2023 14:08

MrSwitch commented Feb 3, 2023

View reviewed changes

chore(ts): improve ts

d18cf50

MrSwitch commented Feb 3, 2023

View reviewed changes

Andrew Dodson added 2 commits February 3, 2023 22:09

chore(typescript): update

aa5d439

chore(typescript): update

8d9919f

MrSwitch commented Feb 6, 2023

View reviewed changes

Andrew Dodson added 3 commits February 6, 2023 14:18

chore(ts): update

6b79ab8

Merge branch 'main' into typescript

b9088c7

chore: tidy

ad47007

diondiondion reviewed Feb 7, 2023

View reviewed changes

Andrew Dodson added 6 commits February 15, 2023 11:50

feat(typescript): pass through ParameterTypes to returned function

7b770a7

BREAKING CHANGE: this changes the getKey parameters 😭

chore(deps): update deps

c938320

feat(ts): add callback values, noissue

d13027d

chore: rm editorconfig

255637e

chore(lint): fix lint

6af0379

chore(ts): remove deprecation warnings

7551e67

MrSwitch merged commit 52fa531 into main Mar 17, 2023

MrSwitch deleted the typescript branch March 17, 2023 12:48

5app-Machine added the released label Mar 17, 2023

	return async (...args: any) => {
	return async (...args: ParameterTypes) => {

feat(typescript): migrate to typescript #56

feat(typescript): migrate to typescript #56

Uh oh!

Conversation

MrSwitch commented Feb 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

5app-Machine commented Mar 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

MrSwitch commented Feb 2, 2023 •

edited

Loading