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

SNOW-595248: Port the library to TypeScript (phase1: add typing) #309

Closed
varun-dc opened this issue May 25, 2022 · 6 comments
Closed

SNOW-595248: Port the library to TypeScript (phase1: add typing) #309

varun-dc opened this issue May 25, 2022 · 6 comments
Assignees
Labels
enhancement The issue is a request for improvement or a new feature status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector.

Comments

@varun-dc
Copy link

varun-dc commented May 25, 2022

This library is written in a old style JavaScript manner. While this is functional for the most part, it would be more maintainable if it was written in a well typed language like TypeScript that would allow better type checking, preventing bugs like #308 (which occurred because of a type mismatch related error)

The library would also be easier for outside contributors to understand as well if they could lean on the TypeScript compiler and associated tooling to navigate and understand the code.

@github-actions github-actions bot changed the title Port the library to TypeScript SNOW-595248: Port the library to TypeScript May 25, 2022
@sfc-gh-jfan sfc-gh-jfan reopened this Jul 1, 2022
@github-actions github-actions bot closed this as completed Jul 2, 2022
@sfc-gh-jfan sfc-gh-jfan reopened this Jul 6, 2022
@kri5t
Copy link

kri5t commented Oct 28, 2022

I completely agree. I was trying to make sense of this library to add functionality of uploading a file from a stream instead of the hard-coded file path upload that is supported. But I gave up because the library was too much a mess.

@sfc-gh-dszmolka sfc-gh-dszmolka added the enhancement The issue is a request for improvement or a new feature label Mar 23, 2023
@sfc-gh-anugupta
Copy link
Collaborator

We plan to add the support for Typescript support, and it's in our backlog. I dont have any ETA yet, but we will prioritize it as we do the next quarter planning. We will keep this thread updated.

Thanks all for your patience.

@dylangrandmont
Copy link

dylangrandmont commented Nov 20, 2023

In case this helps increase the priority of this change, the corresponding type dependency for this library, https://www.npmjs.com/package/@types/snowflake-sdk is quite out of date. For example, there are no typings for parameters, which is important for allowing multi-statement SQL batches on specific queries.

(I understand that typing library is not maintained by Snowflake, but porting the library to TS would remove the need for that dependency entirely and would automatically keep the types and source code in sync)

@sfc-gh-dszmolka
Copy link
Collaborator

sfc-gh-dszmolka commented Feb 8, 2024

Hi everyone - it's been a while but I have some progress to report. We're actively working on adding the typing for snowflake-sdk and then going forward, maintaining it which will allow us to release the changes to the typing alongside with the changes of the driver, if any. Thus hopefully preventing the discrepancy between the changes to the driver library vs. the externally maintained typing lagging behind.

Rewriting the whole library in TypeScript is another story though, I don't have any estimation for that, but the types are coming soon.
edit: PR is #762 - will keep this thread posted with the progress

@sfc-gh-dszmolka sfc-gh-dszmolka added the status-in_progress Issue is worked on by the driver team label Feb 11, 2024
@sfc-gh-dszmolka sfc-gh-dszmolka added status-pr_pending_merge A PR is made and is under review and removed status-in_progress Issue is worked on by the driver team labels May 7, 2024
@sfc-gh-dszmolka sfc-gh-dszmolka changed the title SNOW-595248: Port the library to TypeScript SNOW-595248: Port the library to TypeScript (phase1: add typing) May 7, 2024
@sfc-gh-dszmolka
Copy link
Collaborator

PR #762 has been merged now, and will be released with the next release cycle, towards end of May 2024

@sfc-gh-dszmolka sfc-gh-dszmolka added status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector. and removed status-pr_pending_merge A PR is made and is under review labels May 22, 2024
@sfc-gh-dszmolka
Copy link
Collaborator

released with May 2024 release cycle, version 1.11.0. thank you for your patience here !

Note: if the externally maintained typings might be conflicting now with the Snowflake-internal ones, please try to uninstall the external one / remove it from the dependencies. Also if there are unexpected issues with the typings, do raise a new issue please.

For rewriting and porting the whole library to TypeScript, there is no plans for the foreseeable future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue is a request for improvement or a new feature status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector.
Projects
None yet
Development

No branches or pull requests

6 participants