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

Regression: LDSingleKindContext type not exported in 9.6.1 #626

Closed
klippx opened this issue Oct 16, 2024 · 14 comments
Closed

Regression: LDSingleKindContext type not exported in 9.6.1 #626

klippx opened this issue Oct 16, 2024 · 14 comments
Labels
bug Something isn't working package: sdk/server-node Label for issues affecting the sdk/server-node package.

Comments

@klippx
Copy link

klippx commented Oct 16, 2024

Is this a support request?

No

Describe the bug

My code:

import type { LDSingleKindContext } from '@launchdarkly/node-server-sdk';

This has pretty much always worked, and still works in 9.6.0. But when renovate want to merge a patch update to 9.6.1 then this is no longer available and breaks our build.

To reproduce

I think you can figure it out :)

Expected behavior

All types that were exported before shall still be exported in a patch version, and if you intended to remove LDSingleKindContext then it should go in the major 10.x release.

Logs

Not needed

SDK version

9.6.1

Language version, developer tools

Node v22.9.0
TypeScript 5.6.3

OS/platform

Any OS

Additional context

Not needed

@klippx klippx added bug Something isn't working package: sdk/server-node Label for issues affecting the sdk/server-node package. labels Oct 16, 2024
@kinyoklion
Copy link
Member

Hey @klippx,

Thank you for the report. There isn't any intention to remove this type, so we will check into how it isn't being exported.

Thank you,
Ryan

@kinyoklion
Copy link
Member

kinyoklion commented Oct 16, 2024

Hello @klippx,

This seems to be working fine when I attempt to reproduce. I also see in the .d.ts files that the type is still exported.

My initial suspicion would be perhaps that the other associated packages are not getting their correct patch versions. I would make sure you are not explicitly depending on any of the common packages used by the SDK, for instance.

What specific error does it produce? Does it say it isn't exported?

Failing that if you could provide potentially a tsconfig file. Additionally please let me know if your project is commonjs or esm. The server package itself is still cjs, but we did add ESM support to some common packages, so I would like to validate that.

The typescript code I used:

import { init } from '@launchdarkly/node-server-sdk';
import type { LDSingleKindContext } from '@launchdarkly/node-server-sdk';

// Set sdkKey to your LaunchDarkly SDK key.
const sdkKey = "";

const client = init(sdkKey);

client.once('ready', async function () {
  showMessage("SDK successfully initialized!");
  const context: LDSingleKindContext = {kind: 'user', key: 'bob'};
  const res = await client.variation('my-boolean-flag', context, false);
  console.log("The result", res);
});

And the package.json that went with it:

{
  "name": "hello-node-typescript",
  "version": "1.0.0",
  "description": "Hello LaunchDarkly for Node.js with TypeScript",
  "main": "index.ts",
  "scripts": {
    "start": "ts-node ./index.ts"
  },
  "author": "LaunchDarkly <[email protected]>",
  "license": "Apache-2.0",
  "devDependencies": {
    "@types/node": "*",
    "ts-node": "*",
    "typescript": "5.6.3"
  },
  "dependencies": {
    "@launchdarkly/node-server-sdk": "9.6.1"
  }
}

And the very basic tsconfig.

{
  "compilerOptions": {
    "module": "commonjs",
    "strict": true
  }
}

Thank you,
Ryan

@thyming
Copy link

thyming commented Oct 16, 2024

for what it's worth, i'm also experiencing this.
possibly related to this change? node-server-sdk-v9.6.0...node-server-sdk-v9.6.1#diff-3049555cc0014f748c79249fb17086aad4ef2778fb9a30e34a0228a57400109d

@kinyoklion
Copy link
Member

Hello @thyming,

Would you by chance be able to share your tsconfig?

Also the link may not be taking me to what you intended to point out. Maybe a screenshot of the relevant part of the diff?

Thank you,
Ryan

@kinyoklion
Copy link
Member

Also is it with a dependabot update, such as the original, or from manually updating the version?

@kinyoklion
Copy link
Member

Eventually it does show me this file:
image

Which is not a file which is used in the server-sdk. That said there is a similar change in the main common library.

It generates .cjs and .mjs independently though, and the .d.ts files are the same. If I can figure out what makes your packages have a problem. Either the tsconfig, or your module is a package, etc. Then I could reproduce and see.

@kinyoklion
Copy link
Member

Theoretically this section should cause the tooling to do the correct thing.

  "exports": {
    "types": "./dist/index.d.ts",
    "require": "./dist/index.cjs",
    "import": "./dist/index.mjs"
  },

@thyming
Copy link

thyming commented Oct 16, 2024

the relevant part is:

{
  "extends": "@tsconfig/node20/tsconfig.json"
}

@kinyoklion
Copy link
Member

Ok. Great. Thank you.

@alexframe
Copy link

@kinyoklion, the LDLogger type is also no longer exported in 9.6.1 - is that intentional?

@kinyoklion
Copy link
Member

There are not any removed types in 9.6.1.

The problem here is the way that multi-type bundles work and I am working to fix it at the moment. If module resolution is set to node16 or higher, then types are not working correctly. When a package supports both CJS and ESM the typing files themselves now need to be .d.ts for esm and .d.cts for the commonjs bundle. In testing we were using module resolution 'node'. So we were not aware of this.

Our common library is now being used for client and server and needs ESM to support tree shaking and other features.

@kinyoklion
Copy link
Member

I've set 9.6.0 to be tagged latest in NPM. I am not sure if dependabot is going to respect it, but you should remain on 9.6.0 until the issue is resolved.

Thank you.

@kinyoklion
Copy link
Member

@thyming @klippx @alexframe

I have published a pre-release version:
"@launchdarkly/node-server-sdk": "9.6.2-beta.1"

If one of you could try this and let me know, that would be great. It has resolved the issue when I test locally using @tsconfig/node20 as well as node22. It also works with the legacy node resolution.

Details of the change are in this PR: #627

Sorry for the issue.

Thank you,
Ryan

@kinyoklion
Copy link
Member

Fixed in 9.7.0. Please let me know if you encounter any additional issues.

Thank you,
Ryan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working package: sdk/server-node Label for issues affecting the sdk/server-node package.
Projects
None yet
Development

No branches or pull requests

4 participants