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

Modernize and support functions without arguments #37

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

0x80
Copy link

@0x80 0x80 commented Feb 16, 2024

When I started using this library in my projects, I ran into a few issues:

  1. Memoized functions were forced to have an object of arguments as their function signature. I have some functions that fetch data from the database without any arguments.
  2. The package manifest is not valid for ESM. The types and build steps in my application seemed ok, yet I received runtime errors from a failed default function import. I have fixed the manifest file and replaced (and simplified) the build system using TSUP, because it was not valid for ESM / CJS combination.
  3. When you pass in a Redis client, the library still calls connect() on it. This seems to be a mistake because connect can only be called once, and therefore control should be left to the calling context I think.

In addition

  • I have configured Yarn via corepack for use with newer Node versions. This way the user does not have to have yarn installed globally on their system to run "yarn install"
  • I have made Typescript more strict, which surfaced some mistakes in the code handling prefix string, so I have fixed those
  • Removed some seemingly unnecessary and obsolete things.

Yarn could be configured for v4 instead of v1, but I tried to not touch the lockfile. I did not expect to update other dependencies then, so we might as well configure it to use yarn v4 now...

@0x80 0x80 changed the title Add support ESM and functions without arguments Modernize and support functions without arguments Feb 16, 2024
@0x80 0x80 marked this pull request as ready for review February 16, 2024 15:37
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.

1 participant