-
Notifications
You must be signed in to change notification settings - Fork 31
fix!: Change to ES Modules to improve support of dynamic loading #1011
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
base: main
Are you sure you want to change the base?
Conversation
|
@launchdarkly/browser size report |
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/js-client-sdk size report |
|
@launchdarkly/js-client-sdk-common size report |
| "typescript": "5.1.6" | ||
| }, | ||
| "peerDependencies": { | ||
| "@langchain/community": "^0.2.0 || ^0.3.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The community packages are nested (@langchain/community/chat_models/fireworks vs @langchain/aws) which causes runtime compile errors in Next.js when trying to load our langchain package. It's possible when we upgrade to langchain v1 this issue might go away but I was not able to get it to load without this additional dependency, even if it is not being used.
| "moduleResolution": "node" | ||
| "moduleResolution": "bundler" | ||
| }, | ||
| "include": ["src"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Inconsistent rootDir and include in tsconfig
The rootDir is set to "." while include is set to ["src"], creating a mismatch. This inconsistency can cause TypeScript to generate incorrect output directory structures or have issues with type checking and IDE support. The rootDir should be "./src" to match the include setting, similar to the other AI provider packages in this PR. The comment explaining the "." rootDir was removed because package.json is no longer imported, but the value wasn't updated accordingly.
The dynamic loading of packages was failing in Next.js applications due to treeshaking. Even if you had the package installed, it would be excluded at runtime because it determined it was not needed. Instead of dynamically requiring a package, we explicitly import the package name which helps the process identify packages to include.
Note
Switches AI SDK and providers to dual ESM/CJS outputs with tsup and replaces dynamic require with explicit dynamic imports; updates build configs and tracking metadata.
@launchdarkly/server-sdk-aiand provider packages (server-ai-openai,server-ai-langchain,server-ai-vercel) to dual outputs (esm+cjs) viatsup; addexportsmap,files: ["dist"], and changetypetomodule.moduleESNext/ES2020), modern targets/libs, bundler/node resolution; extend eslint tsconfigs to include*.js.requirewith explicit dynamicimport(...)inapi/providers/AIProviderFactoryand add debug logs for provider creation.LDAIClientImplandJudge.LDLoggerexport to type alias from namespace import for CJS compatibility insrc/index.ts.src/sdkInfo.tswithaiSdkName/aiSdkVersion; use inLDAIConfigTrackerImpland tests.release-please-config.jsonto tracksrc/sdkInfo.tsand example dependencies.tsup.config.tsto SDK and provider packages.Written by Cursor Bugbot for commit 46a645d. This will update automatically on new commits. Configure here.