Skip to content

Conversation

rawpixel-vincent
Copy link

@rawpixel-vincent rawpixel-vincent commented Jun 28, 2025

fix #21

tests pass

it's probably a breaking changes for some projects.

in node 22 using modules

I have to use this to get the proper types:

import * as iovalkey from 'iovalkey';
const IOValkey = iovalkey.default;

// OR

import IOValkey from 'iovalkey';

// new IOValkey() => const IOValkey: new (options: iovalkey.RedisOptions) => Redis (+7 overloads)

haven't tested in commonjs node projects but noticed you support node 18.x so hopefully that still works

@rawpixel-vincent rawpixel-vincent force-pushed the node-next-type-compatibility branch 7 times, most recently from a4b3927 to 1d6dca5 Compare June 28, 2025 09:09
Copy link
Collaborator

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

Good work! I'll ship asap if CI is green

@mcollina
Copy link
Collaborator

mcollina commented Jul 4, 2025

Can you fix CI?

@rawpixel-vincent
Copy link
Author

ok second iteration.. and ran more test locally with different setup.
I have to do a harmless trick to make the type resolution behave correctly when using different moduleResolution/target in the consumer packages tsconfig.

but then the actual code remains the same.

some notes:

✅ types are happy
✅ import works

import IOValkey from 'iovalkey';

✅ types are happy
✅ import works

import * as iovalkey from 'iovalkey';
const IOValkey = iovalkey.default;

✅ types are happy
✅ import works

import { default as IOValkey } from 'iovalkey';

✅ types are happy
✅ import works with the update that add the named exports

import { Redis as IOValkey } from 'iovalkey';
import { Command } from 'iovalkey';

N.B. without named export (using export default as Redis from redis.js..) => import crash => Named export 'Redis' not found. The requested module 'iovalkey' is a CommonJS module, blablabla. CommonJS modules can always be imported via the default export,..: import pkg from 'iovalkey'; const { Redis } = pkg;

Same for all export named as, that's not compatible with es module.

I don't think there's any breaking changes anymore

@rawpixel-vincent rawpixel-vincent force-pushed the node-next-type-compatibility branch from 68a07a4 to 66aed64 Compare July 4, 2025 18:19
@rawpixel-vincent

This comment was marked as resolved.

@rawpixel-vincent rawpixel-vincent marked this pull request as draft July 4, 2025 18:55
@rawpixel-vincent rawpixel-vincent force-pushed the node-next-type-compatibility branch 2 times, most recently from 84e8d5a to 9e7638d Compare July 4, 2025 19:14
@rawpixel-vincent
Copy link
Author

rawpixel-vincent commented Jul 4, 2025

Okay I figured I can run the checks in my fork, all green, had to sort some exports order before types exports in index.ts

@rawpixel-vincent rawpixel-vincent marked this pull request as ready for review July 4, 2025 19:17
@rawpixel-vincent
Copy link
Author

lgtm

Good work! I'll ship asap if CI is green

Thanks, no rush on my end 👌🏻

@rawpixel-vincent rawpixel-vincent force-pushed the node-next-type-compatibility branch from 9e7638d to 46b677c Compare July 4, 2025 19:26
@rawpixel-vincent
Copy link
Author

rawpixel-vincent commented Jul 4, 2025

btw I just saw that the readme has incorrect instruction, probably a search and replace for Redis

// Import iovalkey.
// You can also use import { Valkey } from "iovalkey"
// if your project is a TypeScript project,
// Note that import Valkey from "iovalkey" is still supported,
// but will be deprecated in the next major version.

there's no named { Valkey } export, I can add that. (in addition to the Redis one to not break existing usage)

and it's there #41

@rawpixel-vincent
Copy link
Author

rawpixel-vincent commented Jul 5, 2025

I can put this pr together with #41, #43 and #44 if its preferable, as they are all using this as base.

@rawpixel-vincent rawpixel-vincent force-pushed the node-next-type-compatibility branch from 46b677c to 48de7da Compare July 5, 2025 08:13
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.

TypeScript error when using moduleResolution: node16
2 participants