search

Azure / azure-sdk-for-js

This repository is for active development of the Azure SDK for JavaScript (NodeJS & Browser). For consumers of the SDK we recommend visiting our public developer docs at https://docs.microsoft.com/javascript/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-js.
MIT License
2.07k stars 1.19k forks source link

@azure/openai 2.0.0-beta2 publishes declaration maps pointing to unpublished sources #31497

Open wessberg opened 2 days ago

wessberg commented 2 days ago

Describe the bug All current versions of @azure/openai published on NPM in the 2.0.0 branch (all alpha- and beta builds) include declaration maps (.d.ts.map) files. These are intended to be used in packages that also include their source code in the published package, such that consuming code editors can be taken directly to the original source declaration by via the TypeScript language service. However, @azure/openai 2.0.0-beta2 does not include the src folder that the declaration maps map the sources to.

To Reproduce Steps to reproduce the behavior:

  1. Install @azure/openai (or decompress and inspect the .tar archive packed by NPM)
  2. Observe that declaration maps (.d.ts.map files) are shipped, pointing to source that are not published along with the package.

Expected behavior The package should either not include declaration maps, or include the sources.

Screenshots If applicable, add screenshots to help explain your problem.

Additional context Add any other context about the problem here.

deyaaeldeen commented 2 days ago

Hi @wessberg, thank you for reaching out! Yes, @azure/openai is a companion library to openai and is not exporting runtime JS definitions at the moment but it might in the future. Please checkout the current samples folder for examples of how to use it alongside openai: https://github.com/Azure/azure-sdk-for-js/tree/main/sdk/openai/openai/samples/v2-beta

The package should include actual executable code.

Could you share more about your use case and why is this a concern?

github-actions[bot] commented 2 days ago

Hi @wessberg. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.

wessberg commented 2 days ago

@deyaaeldeen

Sure! As described in the issue description:

As a consequence, the section from the Migration guide from v1 to v2 about how to import and construct an AzureKeyCredential with an api token is currently non-functional, both for JavaScript runtimes, but also in the type layer, as @azure/openai does not export any typings either (only via @azure/openai/types, which the migration guide does not describe).

A reported issue that is related to this is: https://github.com/Azure/azure-sdk-for-js/issues/31170

deyaaeldeen commented 2 days ago

Are you there describing how AzureKeyCredentials were created in the past, and that environment variables should be used going forward

Yes.

wessberg commented 2 days ago

OK. In that case, let me rephrase the original issue to only cover that the package ships declaration maps pointing to sources not shipped alongside the published package.

deyaaeldeen commented 2 days ago

The package should either not include declaration maps, or include the sources.

For context, the package provides partial declaration maps for exclusive features in Azure OpenAI. Importing @azure/openai/types in your code will light up those features in your intellisense using module augmentation. The actual code in the OpenAI client library doesn't need to do anything special to handle requests and responses for those features so no need to ship actual definitions.

I hope this explanation helps clarify the organizational structure of the code in the package. If you have specific needs or questions about your use case, I would be glad to review them to determine if adjustments are necessary. Meanwhile, I encourage you to explore the provided samples.

I am closing this issue for now, but please do not hesitate to reopen it or create new ones if you encounter further questions or issues.

wessberg commented 2 days ago

@deyaaeldeen I fully understand that 👍. I'm not refering to the declaration files, (.d.ts), I'm refering to the declaration maps (.d.ts.map), which are published by your library, because your tsconfig is configured to generate them. But, they are pointing to sources that are not bundled along with the package. In your case, declaration maps are really not relevant, as the sources and dist are effectively the same, considering they're all working with ambient declarations in the type layer.

deyaaeldeen commented 2 days ago

Sorry I missed that earlier; you're right. The maps are great for us to check our code, but they might not mean much to our customers without the src folder included. I'll circle back to you on this soon. Meanwhile, could you let me know if this is causing any problems for you? Thanks for your patience!

wessberg commented 2 days ago

No worries! And no, it's only a minor thing. The Typescript Language Service will still fall back to the .d.ts file for Go To Definition actions if the referenced source file could not be resolved. So really you can just backlog this, or close it again if it provides you value internally.